From 07fcfcccc64f1ca6070c8f4465087ed34d6b20a5 Mon Sep 17 00:00:00 2001 From: Daniel Isaac Khan Ramiro Date: Thu, 28 Oct 2021 21:46:46 -0700 Subject: [PATCH] Deprecate multi-region support on resource name level This feature was introduced in early days of the provider to support multi-region. However, this model was not compatible with Terraform modules since the region was attached to the resource name. Later, multiregion support was added on the provider level as expected by Terraform enabling the provider to be used in modules and enabling users to specify the regions on the provider level. The multi-region support on the resource name level has been still supported till now but it's time to say goodbye to features that had some value in the past but now don't make sense and makes the code more difficult to read and maintain. --- docs/how_to.md | 56 ---- .../swaggercodegen/api/resources/swagger.yaml | 57 ---- openapi/openapi_v2_resource.go | 27 +- openapi/openapi_v2_resource_test.go | 121 +------ openapi/openapi_v2_spec_analyser.go | 73 ---- openapi/openapi_v2_spec_analyser_test.go | 317 ------------------ tests/integration/resource_monitors_test.go | 59 ---- 7 files changed, 12 insertions(+), 698 deletions(-) diff --git a/docs/how_to.md b/docs/how_to.md index 89a168904..dce4de027 100644 --- a/docs/how_to.md +++ b/docs/how_to.md @@ -522,7 +522,6 @@ Extension Name | Type | Description [x-terraform-resource-poll-enabled](#xTerraformResourcePollEnabled) | bool | Only supported in operation responses (e,g: 202). Defines that if the API responds with the given HTTP Status code (e,g: 202), the polling mechanism will be enabled. This allows the OpenAPI Terraform provider to perform read calls to the remote API and check the resource state. The polling mechanism finalises if the remote resource state arrives at completion, failure state or times-out (60s) [x-terraform-resource-name](#xTerraformResourceName) | string | Only supported in resource root level. Defines the name that will be used for the resource in the Terraform configuration. If the extension is not preset, default value will be the name of the resource in the path. For instance, a path such as /v1/users will translate into a terraform resource name users_v1 [x-terraform-resource-host](#xTerraformResourceHost) | string | Only supported in resource root's POST operation. Defines the host that should be used when managing this specific resource. The value of this extension effectively overrides the global host configuration, making the OpenAPI Terraform provider client make thje API calls against the host specified in this extension value instead of the global host configuration. The protocols (HTTP/HTTPS) and base path (if anything other than "/") used when performing the API calls will still come from the global configuration. -[x-terraform-resource-regions-%s](#xTerraformResourceRegions) | string | Only supported in the root level. Defines the regions supported by a given resource identified by the %s variable. This extension only works if the ```x-terraform-resource-host``` extension contains a value that is parametrized and identifies the matching ```x-terraform-resource-regions-%s``` extension. The values of this extension must be comma separated strings. ###### x-terraform-exclude-resource @@ -793,61 +792,6 @@ the overridden host instead, in this case ```cdn.api.otherdomain.com```. *Note: This extension is only supported at the operation's POST operation level. The other operations available for the resource such as GET/PUT/DELETE will used the overridden host value too.* -###### Multi-region resources - -Additionally, if the resource is using multi region domains, meaning there's one sub-domain for each region where the resource -can be created into (similar to how aws resources are created per region), this can be configured as follows: - -```` -swagger: "2.0" -host: "some.domain.com" -x-terraform-resource-regions-cdn: "dub1,sea1" -paths: - /v1/cdns: - post: - x-terraform-resource-host: cdn.${cdn}.api.otherdomain.com -```` - -If the ``x-terraform-resource-host`` extension has a value parameterised in the form where the following pattern ```${identifier}``` - is found (identifier being any string with no whitspaces - spaces,tabs, line breaks, etc) AND there is a matching - extension 'x-terraform-resource-regions-**identifier**' defined in the root level that refers to the same identifier - then the resource will be considered multi region. -For instance, in the above example, the ```x-terraform-resource-host``` value is parameterised as the ```${identifier}``` pattern -is found, and the identifier in this case is ```cdn```. Moreover, there is a matching ```x-terraform-resource-regions-cdn``` -extension containing a list of regions where this resource can be created in. - -The regions found in the ```x-terraform-resource-regions-cdn``` will be used as follows: - -- The OpenAPI Terraform provider will expose one resource per region enlisted in the extension. In the case above, the -following resources will become available in the Terraform configuration (the provider name chosen here is 'swaggercodegen'): - -```` -resource "swaggercodegen_cdn_v1_dub1" "my_cdn" { - label = "label" - ips = ["127.0.0.1"] - hostnames = ["origin.com"] -} - -resource "swaggercodegen_cdn_v1_sea1" "my_cdn" { - label = "label" - ips = ["127.0.0.1"] - hostnames = ["origin.com"] -} -```` - -As shown above, the resources that are multi-region will have extra information in their name that identifies the region -where tha resource should be managed. - -- The OpenAPI Terraform provider client will make the API call against the specific resource region when the resource -is configured with multi-region support. - -- As far as the resource configuration is concerned, the swagger configuration remains the same for that specific resource -(parameters, operations, polling support, etc) and the same configuration will be applicable to all the regions that resource -supports. - -*Note: This extension is only supported at the root level and can be used exclusively along with the 'x-terraform-resource-host' -extension* - #### Definitions - **Field Name:** definitions diff --git a/examples/swaggercodegen/api/resources/swagger.yaml b/examples/swaggercodegen/api/resources/swagger.yaml index 9e0b3654b..850261c07 100644 --- a/examples/swaggercodegen/api/resources/swagger.yaml +++ b/examples/swaggercodegen/api/resources/swagger.yaml @@ -42,9 +42,6 @@ security: x-terraform-provider-multiregion-fqdn: "some.api.${region}.nonexistingrandomdomain.io" # Making it a bit more random to avoid resolving to an actual existing domain x-terraform-provider-regions: "rst1,dub1" -# This is legacy configuration that will be deprecated soon -x-terraform-resource-regions-monitor: "rst1,dub1" - paths: /swagger.json: get: @@ -334,60 +331,6 @@ paths: 404: $ref: "#/responses/NotFound" - - ############################ - ##### Monitors Multiregion Resource name based #### - ############################ - - # The monitor resource is not implemented in the backed, it only serves as an example on how the global host can be overridden - # and how the resource can be configured with multi region setup - - /v1/monitors: - post: - tags: - - "monitor" - summary: "Create monitor v1" - operationId: "CreateMonitorV1" - x-terraform-resource-host: "some.api.${monitor}.nonexistingrandomdomain.io" # Making it a bit more random to avoid resolving to an actual existing domain - parameters: - - in: "body" - name: "body" - description: "Monitor v1 payload object to be posted as part of the POST request" - required: true - schema: - $ref: "#/definitions/MonitorV1" - responses: - 200: - description: "this operation is asynchronous, to check the status of the deployment call GET operation and check the status field returned in the payload" - schema: - $ref: "#/definitions/MonitorV1" - default: - description: "generic error response" - schema: - $ref: "#/definitions/Error" - /v1/monitors/{id}: - get: - tags: - - "monitor" - summary: "Get monitor by id" - description: "" - operationId: "GetMonitorV1" - parameters: - - name: "id" - in: "path" - description: "The monitor v1 id that needs to be fetched." - required: true - type: "string" - responses: - 200: - description: "successful operation" - schema: - $ref: "#/definitions/MonitorV1" - 400: - description: "Invalid monitor id supplied" - 404: - description: "Monitor not found" - ############################ ##### Monitors MultiRegion Resource #### ############################ diff --git a/openapi/openapi_v2_resource.go b/openapi/openapi_v2_resource.go index 8699e4198..515fe40f3 100644 --- a/openapi/openapi_v2_resource.go +++ b/openapi/openapi_v2_resource.go @@ -70,8 +70,7 @@ const extTfResourceURL = "x-terraform-resource-host" // SpecV2Resource defines a struct that implements the SpecResource interface and it's based on OpenAPI v2 specification type SpecV2Resource struct { - Name string - Region string + Name string // Path contains the full relative path to the resource e,g: /v1/resource Path string // SpecSchemaDefinition definition represents the representational state (aka model) of the resource @@ -100,13 +99,12 @@ type SpecV2Resource struct { // newSpecV2Resource creates a SpecV2Resource with no region and default host func newSpecV2Resource(path string, schemaDefinition spec.Schema, rootPathItem, instancePathItem spec.PathItem, schemaDefinitions map[string]spec.Schema, paths map[string]spec.PathItem) (*SpecV2Resource, error) { - return newSpecV2ResourceWithConfig("", path, schemaDefinition, rootPathItem, instancePathItem, schemaDefinitions, paths) + return newSpecV2ResourceWithConfig(path, schemaDefinition, rootPathItem, instancePathItem, schemaDefinitions, paths) } func newSpecV2DataSource(path string, schemaDefinition spec.Schema, rootPathItem spec.PathItem, paths map[string]spec.PathItem) (*SpecV2Resource, error) { resource := &SpecV2Resource{ Path: path, - Region: "", SchemaDefinition: schemaDefinition, RootPathItem: rootPathItem, InstancePathItem: spec.PathItem{}, @@ -121,15 +119,7 @@ func newSpecV2DataSource(path string, schemaDefinition spec.Schema, rootPathItem return resource, nil } -// newSpecV2ResourceWithRegion creates a SpecV2Resource with the region configured making the returned SpecV2Resource region based. -func newSpecV2ResourceWithRegion(region, path string, schemaDefinition spec.Schema, rootPathItem, instancePathItem spec.PathItem, schemaDefinitions map[string]spec.Schema, paths map[string]spec.PathItem) (*SpecV2Resource, error) { - if region == "" { - return nil, fmt.Errorf("region must not be empty") - } - return newSpecV2ResourceWithConfig(region, path, schemaDefinition, rootPathItem, instancePathItem, schemaDefinitions, paths) -} - -func newSpecV2ResourceWithConfig(region, path string, schemaDefinition spec.Schema, rootPathItem, instancePathItem spec.PathItem, schemaDefinitions map[string]spec.Schema, paths map[string]spec.PathItem) (*SpecV2Resource, error) { +func newSpecV2ResourceWithConfig(path string, schemaDefinition spec.Schema, rootPathItem, instancePathItem spec.PathItem, schemaDefinitions map[string]spec.Schema, paths map[string]spec.PathItem) (*SpecV2Resource, error) { if path == "" { return nil, fmt.Errorf("path must not be empty") } @@ -138,7 +128,6 @@ func newSpecV2ResourceWithConfig(region, path string, schemaDefinition spec.Sche } resource := &SpecV2Resource{ Path: path, - Region: region, SchemaDefinition: schemaDefinition, RootPathItem: rootPathItem, InstancePathItem: instancePathItem, @@ -155,9 +144,6 @@ func newSpecV2ResourceWithConfig(region, path string, schemaDefinition spec.Sche // GetResourceName returns the resource name including the region at the end of the resource name if applicable func (o *SpecV2Resource) GetResourceName() string { - if o.Region != "" { - return fmt.Sprintf("%s_%s", o.Name, o.Region) - } return o.Name } @@ -262,13 +248,6 @@ func (o *SpecV2Resource) getHost() (string, error) { if overrideHost == "" { return "", nil } - multiRegionHost, err := openapiutils.GetMultiRegionHost(overrideHost, o.Region) - if err != nil { - return "", err - } - if multiRegionHost != "" { - return multiRegionHost, nil - } return overrideHost, nil } diff --git a/openapi/openapi_v2_resource_test.go b/openapi/openapi_v2_resource_test.go index ab5e9a359..4a6cc86c8 100644 --- a/openapi/openapi_v2_resource_test.go +++ b/openapi/openapi_v2_resource_test.go @@ -43,7 +43,7 @@ func TestNewSpecV2ResourceWithConfig(t *testing.T) { } Convey("When newSpecV2ResourceWithConfig method is called", func() { schemaDefinitions := map[string]spec.Schema{} - r, err := newSpecV2ResourceWithConfig("", path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) + r, err := newSpecV2ResourceWithConfig(path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) Convey("Then the result returned should be the expected one", func() { So(err, ShouldBeNil) So(r.GetResourceName(), ShouldEqual, "users") @@ -59,31 +59,13 @@ func TestNewSpecV2ResourceWithConfig(t *testing.T) { } Convey("When newSpecV2ResourceWithConfig method is called", func() { schemaDefinitions := map[string]spec.Schema{} - r, err := newSpecV2ResourceWithConfig("", path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) + r, err := newSpecV2ResourceWithConfig(path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) Convey("Then the result returned should be the expected one", func() { So(err, ShouldBeNil) So(r.GetResourceName(), ShouldEqual, "users") }) }) }) - Convey("Given a root path /users, a root path item and a region", t, func() { - path := "/users" - rootPathItem := spec.PathItem{ - PathItemProps: spec.PathItemProps{ - Post: &spec.Operation{}, - }, - } - region := "rst1" - Convey("When newSpecV2ResourceWithConfig method is called", func() { - schemaDefinitions := map[string]spec.Schema{} - r, err := newSpecV2ResourceWithConfig(region, path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) - Convey("Then the result returned should be the expected one", func() { - So(err, ShouldBeNil) - // the value returned should be the resource name plus the region appended at the end - So(r.GetResourceName(), ShouldEqual, "users_rst1") - }) - }) - }) Convey("Given a root path that is versioned such as '/v1/users/' and a root path item item", t, func() { path := "/v1/users/" rootPathItem := spec.PathItem{ @@ -93,7 +75,7 @@ func TestNewSpecV2ResourceWithConfig(t *testing.T) { } Convey("When newSpecV2ResourceWithConfig method is called", func() { schemaDefinitions := map[string]spec.Schema{} - r, err := newSpecV2ResourceWithConfig("", path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) + r, err := newSpecV2ResourceWithConfig(path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) Convey("Then the error returned should be nil", func() { So(err, ShouldBeNil) So(r.GetResourceName(), ShouldEqual, "users_v1") @@ -109,7 +91,7 @@ func TestNewSpecV2ResourceWithConfig(t *testing.T) { } Convey("When newSpecV2ResourceWithConfig method is called", func() { schemaDefinitions := map[string]spec.Schema{} - r, err := newSpecV2ResourceWithConfig("", path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) + r, err := newSpecV2ResourceWithConfig(path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) Convey("Then the result returned should be the expected one", func() { So(err, ShouldBeNil) So(r.GetResourceName(), ShouldEqual, "users_v12") @@ -125,7 +107,7 @@ func TestNewSpecV2ResourceWithConfig(t *testing.T) { } Convey("When newSpecV2ResourceWithConfig method is called", func() { schemaDefinitions := map[string]spec.Schema{} - r, err := newSpecV2ResourceWithConfig("", path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) + r, err := newSpecV2ResourceWithConfig(path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) Convey("Then the result returned should be the expected one", func() { So(err, ShouldBeNil) So(r.GetResourceName(), ShouldEqual, "users") @@ -148,7 +130,7 @@ func TestNewSpecV2ResourceWithConfig(t *testing.T) { } Convey("When newSpecV2ResourceWithConfig method is called", func() { schemaDefinitions := map[string]spec.Schema{} - r, err := newSpecV2ResourceWithConfig("", path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, paths) + r, err := newSpecV2ResourceWithConfig(path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, paths) Convey("Then the result returned should be the expected one", func() { So(err, ShouldBeNil) So(r.GetResourceName(), ShouldEqual, "nodes_v1_proxy") @@ -171,7 +153,7 @@ func TestNewSpecV2ResourceWithConfig(t *testing.T) { } Convey("When newSpecV2ResourceWithConfig method is called", func() { schemaDefinitions := map[string]spec.Schema{} - r, err := newSpecV2ResourceWithConfig("", path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) + r, err := newSpecV2ResourceWithConfig(path, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) resourceName := r.GetResourceName() expectedTerraformName := fmt.Sprintf("%s_v1", expectedResourceName) Convey(fmt.Sprintf("And the value returned should still be '%s'", expectedTerraformName), func() { @@ -189,7 +171,7 @@ func TestNewSpecV2ResourceWithConfig(t *testing.T) { } Convey("When newSpecV2ResourceWithConfig method is called", func() { schemaDefinitions := map[string]spec.Schema{} - _, err := newSpecV2ResourceWithConfig("", invalidRootPath, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) + _, err := newSpecV2ResourceWithConfig(invalidRootPath, spec.Schema{}, rootPathItem, spec.PathItem{}, schemaDefinitions, map[string]spec.PathItem{}) Convey("And the err returned should not be nil", func() { So(err, ShouldNotBeNil) }) @@ -210,7 +192,7 @@ func TestNewSpecV2ResourceWithConfig(t *testing.T) { var paths map[string]spec.PathItem Convey("When newSpecV2ResourceWithConfig method is called", func() { schemaDefinitions := map[string]spec.Schema{} - r, err := newSpecV2ResourceWithConfig("", "/v1/users", spec.Schema{}, spec.PathItem{}, spec.PathItem{}, schemaDefinitions, paths) + r, err := newSpecV2ResourceWithConfig("/v1/users", spec.Schema{}, spec.PathItem{}, spec.PathItem{}, schemaDefinitions, paths) Convey("And the err returned output should match the expectation", func() { So(err.Error(), ShouldEqual, "paths must not be nil") So(r, ShouldBeNil) @@ -219,45 +201,6 @@ func TestNewSpecV2ResourceWithConfig(t *testing.T) { }) } -func TestNewSpecV2ResourceWithRegion(t *testing.T) { - Convey("Given a path, schemaDefinition, rootPathItem, instancePathItem, paths, schemaDefinitions AND a region that is empty", t, func() { - path := "/v1/users" - schemaDefinition := spec.Schema{} - rootPathItem := spec.PathItem{} - instancePathItem := spec.PathItem{} - paths := map[string]spec.PathItem{} - schemaDefinitions := map[string]spec.Schema{} - region := "" - Convey("When newSpecV2ResourceWithRegion method is called", func() { - r, err := newSpecV2ResourceWithRegion(region, path, schemaDefinition, rootPathItem, instancePathItem, schemaDefinitions, paths) - Convey("Then the result returned should be the expected one", func() { - So(err.Error(), ShouldEqual, "region must not be empty") - So(r, ShouldBeNil) - }) - }) - }) - Convey("Given a path, schemaDefinition, rootPathItem, instancePathItem, paths, schemaDefinitions AND a region that is NOT empty", t, func() { - path := "/users" - rootPathItem := spec.PathItem{ - PathItemProps: spec.PathItemProps{ - Post: &spec.Operation{}, - }, - } - schemaDefinition := spec.Schema{} - instancePathItem := spec.PathItem{} - paths := map[string]spec.PathItem{} - schemaDefinitions := map[string]spec.Schema{} - region := "rst1" - Convey("When newSpecV2ResourceWithRegion method is called", func() { - r, err := newSpecV2ResourceWithRegion(region, path, schemaDefinition, rootPathItem, instancePathItem, schemaDefinitions, paths) - Convey("Then the value returned should be the resource name plus the region appended at the end", func() { - So(err, ShouldBeNil) - So(r.GetResourceName(), ShouldEqual, "users_rst1") - }) - }) - }) -} - func TestShouldIgnoreResource(t *testing.T) { Convey("Given a SpecV2Resource configured with a root path item that does not contain the post operation defined", t, func() { r := SpecV2Resource{ @@ -3952,52 +3895,6 @@ func TestSpecV2ResourceGetHost(t *testing.T) { }) }) }) - Convey("Given a SpecV2Resource that is multi region but region isn't specified", t, func() { - r := SpecV2Resource{ - Region: "", - RootPathItem: spec.PathItem{ - PathItemProps: spec.PathItemProps{ - Post: &spec.Operation{ - VendorExtensible: spec.VendorExtensible{ - Extensions: spec.Extensions{ - extTfResourceURL: "www.${region}.some-host.com", - }, - }, - }, - }, - }, - } - Convey("When getHost is called", func() { - host, err := r.getHost() - Convey("Then the host returned should be an empty string", func() { - So(err.Error(), ShouldEqual, "region can not be empty for multiregion resources") - So(host, ShouldBeEmpty) - }) - }) - }) - Convey("Given a SpecV2Resource that is multi region with region specified", t, func() { - r := SpecV2Resource{ - Region: "rst1", - RootPathItem: spec.PathItem{ - PathItemProps: spec.PathItemProps{ - Post: &spec.Operation{ - VendorExtensible: spec.VendorExtensible{ - Extensions: spec.Extensions{ - extTfResourceURL: "www.${region}.some-host.com", - }, - }, - }, - }, - }, - } - Convey("When getHost is called", func() { - host, err := r.getHost() - Convey("Then the host returned should be the override host", func() { - So(err, ShouldBeNil) - So(host, ShouldEqual, "www.rst1.some-host.com") - }) - }) - }) } func TestGetResourceOverrideHost(t *testing.T) { diff --git a/openapi/openapi_v2_spec_analyser.go b/openapi/openapi_v2_spec_analyser.go index 4c67eb053..98ac5e3be 100644 --- a/openapi/openapi_v2_spec_analyser.go +++ b/openapi/openapi_v2_spec_analyser.go @@ -9,13 +9,10 @@ import ( "strings" "time" - "github.com/dikhan/terraform-provider-openapi/v2/openapi/openapiutils" "github.com/go-openapi/loads" "github.com/go-openapi/spec" ) -const extTfResourceRegionsFmt = "x-terraform-resource-regions-%s" - // specV2Analyser defines an SpecAnalyser implementation for OpenAPI v2 specification // Forcing creation of this object via constructor so proper input validation is performed before creating the struct // instance @@ -44,19 +41,6 @@ func newSpecAnalyserV2(openAPIDocumentFilename string) (*specV2Analyser, error) }, nil } -func (specAnalyser *specV2Analyser) createMultiRegionResources(regions []string, resourceRootPath string, resourceRoot, pathItem spec.PathItem, resourcePayloadSchemaDef *spec.Schema) ([]SpecResource, error) { - var resources []SpecResource - for _, regionName := range regions { - r, err := newSpecV2ResourceWithRegion(regionName, resourceRootPath, *resourcePayloadSchemaDef, resourceRoot, pathItem, specAnalyser.d.Spec().Definitions, specAnalyser.d.Spec().Paths.Paths) - if err != nil { - return nil, fmt.Errorf("failed to create a resource with region: %s", err) - } - log.Printf("[INFO] multi region resource name = %s, region = '%s'", r.GetResourceName(), regionName) - resources = append(resources, r) - } - return resources, nil -} - func (specAnalyser *specV2Analyser) GetTerraformCompliantDataSources() []SpecResource { var dataSources []SpecResource spec := specAnalyser.d.Spec() @@ -92,22 +76,6 @@ func (specAnalyser *specV2Analyser) GetTerraformCompliantResources() ([]SpecReso continue } - isMultiRegion, regions, err := specAnalyser.isMultiRegionResource(resourceRoot, specAnalyser.d.Spec().Extensions) - if err != nil { - log.Printf("multi region configuration for resource '%s' is not valid: ", err) - continue - } - if isMultiRegion { - log.Printf("[INFO] resource '%s' is configured with host override AND multi region; creating one reasource per region", resourceRootPath) - multiRegionResources, err := specAnalyser.createMultiRegionResources(regions, resourceRootPath, *resourceRoot, pathItem, resourcePayloadSchemaDef) - if err != nil { - log.Printf("[WARN] ignoring multiregion resource '%s' due to an error: %s", resourceRootPath, err) - continue - } - resources = append(resources, multiRegionResources...) - continue - } - r, err := newSpecV2Resource(resourceRootPath, *resourcePayloadSchemaDef, *resourceRoot, pathItem, specAnalyser.d.Spec().Definitions, specAnalyser.d.Spec().Paths.Paths) if err != nil { log.Printf("[WARN] ignoring resource '%s' due to an error while creating a creating the SpecV2Resource: %s", resourceRootPath, err) @@ -162,47 +130,6 @@ func (specAnalyser *specV2Analyser) pathExists(path string) (bool, spec.PathItem return true, p } -// isMultiRegionResource returns true on ly if: -// - the value is parametrized following the pattern: some.subdomain.${keyword}.domain.com, where ${keyword} must be present in the string, otherwise the resource will not be considered multi region -// - there is a matching 'x-terraform-resource-regions-${keyword}' extension defined in the swagger root level (extensions passed in), where ${keyword} will be the value of the parameter in the above URL -// - and finally the value of the extension is an array of strings containing the different regions where the resource can be created -func (specAnalyser *specV2Analyser) isMultiRegionResource(resourceRoot *spec.PathItem, extensions spec.Extensions) (bool, []string, error) { - overrideHost := getResourceOverrideHost(resourceRoot.Post) - if overrideHost == "" { - return false, nil, nil - } - isMultiRegionHost, regex := openapiutils.IsMultiRegionHost(overrideHost) - if !isMultiRegionHost { - return false, nil, nil - } - region := regex.FindStringSubmatch(overrideHost) - if len(region) != 5 { - return false, nil, fmt.Errorf("override host %s provided does not comply with expected regex format", overrideHost) - } - regionIdentifier := region[3] - regionExtensionName := specAnalyser.getResourceRegionExtensionName(regionIdentifier) - if resourceRegions, exists := openapiutils.StringExtensionExists(extensions, regionExtensionName); exists { - resourceRegions = strings.Replace(resourceRegions, " ", "", -1) - regions := strings.Split(resourceRegions, ",") - if len(regions) < 1 { - return false, nil, fmt.Errorf("could not find any region for '%s' matching region extension %s: '%s'", regionIdentifier, regionExtensionName, resourceRegions) - } - apiRegions := []string{} - for _, region := range regions { - apiRegions = append(apiRegions, region) - } - if len(apiRegions) < 1 { - return false, nil, fmt.Errorf("could not build properly the resource region map for '%s' matching region extension %s: '%s'", regionIdentifier, regionExtensionName, resourceRegions) - } - return true, apiRegions, nil - } - return false, nil, fmt.Errorf("missing matching '%s' root level region extension '%s'", regionIdentifier, regionExtensionName) -} - -func (specAnalyser *specV2Analyser) getResourceRegionExtensionName(regionIdentifier string) string { - return fmt.Sprintf(extTfResourceRegionsFmt, regionIdentifier) -} - func (specAnalyser *specV2Analyser) GetSecurity() SpecSecurity { return &specV2Security{ SecurityDefinitions: specAnalyser.d.Spec().SecurityDefinitions, diff --git a/openapi/openapi_v2_spec_analyser_test.go b/openapi/openapi_v2_spec_analyser_test.go index dfdf8afe3..816366dbd 100644 --- a/openapi/openapi_v2_spec_analyser_test.go +++ b/openapi/openapi_v2_spec_analyser_test.go @@ -1968,70 +1968,6 @@ func TestGetAPIBackendConfiguration(t *testing.T) { }) } -func TestIsMultiRegionResource(t *testing.T) { - Convey("Given a specV2Analyser and a resource root has a POST operation containing the x-terraform-resource-host with a parametrized host containing region variable", t, func() { - serviceProviderName := "serviceProviderName" - r := specV2Analyser{} - resourceRoot := &spec.PathItem{ - PathItemProps: spec.PathItemProps{ - Post: &spec.Operation{ - VendorExtensible: spec.VendorExtensible{ - Extensions: spec.Extensions{ - extTfResourceURL: fmt.Sprintf("some.api.${%s}.domain.com", serviceProviderName), - }, - }, - }, - }, - } - Convey("When isMultiRegionResource method is called with a resourceRoot pathItem and a set of extensions where one matches the region for which the above 's-terraform-resource-host' extension is for", func() { - rootLevelExtensions := spec.Extensions{} - rootLevelExtensions.Add(fmt.Sprintf(extTfResourceRegionsFmt, serviceProviderName), "uswest,useast") - isMultiRegion, regions, err := r.isMultiRegionResource(resourceRoot, rootLevelExtensions) - Convey("Then the list returned should contain uswest and useast", func() { - So(err, ShouldBeNil) - So(isMultiRegion, ShouldBeTrue) - So(regions, ShouldContain, "uswest") - So(regions, ShouldContain, "useast") - }) - }) - - Convey("When isMultiRegionResource method is called with a set of extensions where NONE matches the region for which the above 's-terraform-resource-host' extension is for", func() { - rootLevelExtensions := spec.Extensions{} - rootLevelExtensions.Add(fmt.Sprintf(extTfResourceRegionsFmt, "someOtherServiceProvider"), "rst, dub") - isMultiRegion, regions, err := r.isMultiRegionResource(resourceRoot, rootLevelExtensions) - Convey("Then the result returned should be the expected one", func() { - So(err, ShouldNotBeNil) - So(err.Error(), ShouldEqual, "missing matching 'serviceProviderName' root level region extension 'x-terraform-resource-regions-serviceProviderName'") - So(isMultiRegion, ShouldBeFalse) - So(regions, ShouldBeEmpty) - }) - }) - - Convey("When isMultiRegionResource method is called with a set of extensions where one matches the region for which the above 's-terraform-resource-host' extension is for BUT the values are not comma separated", func() { - rootLevelExtensions := spec.Extensions{} - rootLevelExtensions.Add(fmt.Sprintf(extTfResourceRegionsFmt, serviceProviderName), "uswest useast") - isMultiRegion, regions, err := r.isMultiRegionResource(resourceRoot, rootLevelExtensions) - Convey("Then the result returned should be the expected one", func() { - So(err, ShouldBeNil) - So(isMultiRegion, ShouldBeTrue) - So(regions, ShouldContain, "uswestuseast") - }) - }) - - Convey("When isMultiRegionResource method is called with a set of extensions where one matches the region for which the above 's-terraform-resource-host' extension is for BUT the values are comma separated with spaces", func() { - rootLevelExtensions := spec.Extensions{} - rootLevelExtensions.Add(fmt.Sprintf(extTfResourceRegionsFmt, serviceProviderName), "uswest, useast") - isMultiRegion, regions, err := r.isMultiRegionResource(resourceRoot, rootLevelExtensions) - Convey("Then the result returned should be the expected one", func() { - So(err, ShouldBeNil) - So(isMultiRegion, ShouldBeTrue) - So(regions, ShouldContain, "uswest") - So(regions, ShouldContain, "useast") - }) - }) - }) -} - func TestResourceInstanceEndPoint(t *testing.T) { Convey("Given an specV2Analyser", t, func() { a := specV2Analyser{} @@ -3765,77 +3701,6 @@ paths: }) } -func TestCreateMultiRegionResources(t *testing.T) { - Convey("Given an specV2Analyser loaded with a swagger file containing a multiregion resource", t, func() { - swaggerContent := `swagger: "2.0" -x-terraform-resource-regions-keyword: "rst1" -paths: - /v1/cdns: - post: - x-terraform-resource-host: some.subdomain.${keyword}.domain.com - parameters: - - in: "body" - name: "body" - schema: - $ref: "#/definitions/ContentDeliveryNetwork" - responses: - 201: - schema: - $ref: "#/definitions/ContentDeliveryNetwork" - /v1/cdns/{id}: - get: - parameters: - - name: "id" - in: "path" - description: "The cdn id that needs to be fetched." - required: true - type: "string" - responses: - 200: - schema: - $ref: "#/definitions/ContentDeliveryNetwork" -definitions: - ContentDeliveryNetwork: - type: "object" - properties: - id: - type: "string" - readOnly: true` - a := initAPISpecAnalyser(swaggerContent) - Convey("When createMultiRegionResources method is called with a map of regions and corresponding resolved URLs", func() { - regions := []string{"rst1"} - resourceRootPath := "/v1/cdns" - pathRootItem := a.d.Spec().Paths.Paths["/v1/cdns"] - pathItem := a.d.Spec().Paths.Paths["/v1/cdns/{id}"] - resourcePayloadSchemaDef := a.d.Spec().Definitions["ContentDeliveryNetwork"] - multiRegionResources, err := a.createMultiRegionResources(regions, resourceRootPath, pathRootItem, pathItem, &resourcePayloadSchemaDef) - Convey("Then the result returned should be the expected one", func() { - So(err, ShouldBeNil) - // the list resources return should only contain a resource called cdns_v1_rst1 - So(len(multiRegionResources), ShouldEqual, 1) - So(multiRegionResources[0].GetResourceName(), ShouldEqual, "cdns_v1_rst1") - cdnMultiRegionResource := multiRegionResources[0] - // the host is correctly configured according to the swagger - host, err := cdnMultiRegionResource.getHost() - So(err, ShouldBeNil) - So(host, ShouldEqual, "some.subdomain.rst1.domain.com") - }) - }) - Convey("When createMultiRegionResources method is called with a map of regions and an empty resourceRootPath", func() { - regions := []string{"rst1"} - resourceRootPath := "" - pathRootItem := a.d.Spec().Paths.Paths["/v1/cdns"] - pathItem := a.d.Spec().Paths.Paths["/v1/cdns/{id}"] - resourcePayloadSchemaDef := a.d.Spec().Definitions["ContentDeliveryNetwork"] - multiRegionResources, err := a.createMultiRegionResources(regions, resourceRootPath, pathRootItem, pathItem, &resourcePayloadSchemaDef) - Convey("Then the error returned should be as expected", func() { - So(err.Error(), ShouldEqual, "failed to create a resource with region: path must not be empty") - So(multiRegionResources, ShouldBeNil) - }) - }) - }) -} - func TestGetTerraformCompliantDataSources(t *testing.T) { testCases := []struct { name string @@ -4263,80 +4128,6 @@ definitions: }) }) - Convey("Given an specV2Analyser loaded with a swagger file containing a compliant terraform resource /v1/cdns that is multi region", t, func() { - swaggerContent := `swagger: "2.0" -x-terraform-resource-regions-keyword: "sea1" -paths: - /v1/cdns: - post: - x-terraform-resource-host: some.subdomain.${keyword}.domain.com - parameters: - - in: "body" - name: "body" - schema: - $ref: "#/definitions/ContentDeliveryNetwork" - responses: - 201: - schema: - $ref: "#/definitions/ContentDeliveryNetwork" - /v1/cdns/{id}: - get: - parameters: - - name: "id" - in: "path" - description: "The cdn id that needs to be fetched." - required: true - type: "string" - responses: - 200: - schema: - $ref: "#/definitions/ContentDeliveryNetwork" -definitions: - ContentDeliveryNetwork: - type: "object" - properties: - id: - type: "string" - readOnly: true` - a := initAPISpecAnalyser(swaggerContent) - Convey("When GetTerraformCompliantResources method is called ", func() { - terraformCompliantResources, err := a.GetTerraformCompliantResources() - Convey("Then the result returned should be the expected one", func() { - So(err, ShouldBeNil) - // the resources info map should only contain a resource called cdns_v1_sea1 - So(len(terraformCompliantResources), ShouldEqual, 1) - So(terraformCompliantResources[0].GetResourceName(), ShouldEqual, "cdns_v1_sea1") - cndV1Resource := terraformCompliantResources[0] - // the cndV1Resource should not be considered a subresource - subRes := cndV1Resource.GetParentResourceInfo() - So(err, ShouldBeNil) - So(subRes, ShouldBeNil) - // the resource operations are attached to the resource schema (GET,POST,PUT,DELETE) as stated in the YAML - resOperation := cndV1Resource.getResourceOperations() - So(resOperation.Get.responses, ShouldContainKey, 200) - So(resOperation.Post.responses, ShouldContainKey, 201) - So(resOperation.Put, ShouldBeNil) - So(resOperation.Delete, ShouldBeNil) - // each operation exposed on the resource has a nil timeout - timeoutSpec, err := cndV1Resource.getTimeouts() - So(err, ShouldBeNil) - So(timeoutSpec.Post, ShouldBeNil) - So(timeoutSpec.Get, ShouldBeNil) - So(timeoutSpec.Put, ShouldBeNil) - So(timeoutSpec.Delete, ShouldBeNil) - // the host is correctly configured according to the swagger - host, err := cndV1Resource.getHost() - So(err, ShouldBeNil) - So(host, ShouldEqual, "some.subdomain.sea1.domain.com") - // the resource schema contains the one property specified in the ContentDeliveryNetwork model definition - actualResourceSchema, err := cndV1Resource.GetResourceSchema() - So(err, ShouldBeNil) - So(len(actualResourceSchema.Properties), ShouldEqual, 1) - So(actualResourceSchema.Properties[0].Name, ShouldEqual, "id") - }) - }) - }) - Convey("Given an specV2Analyser loaded with a swagger file containing a compliant terraform resource /v1/cdns with a POST's request payload model without the id property and the returned payload and the GET operation have it'", t, func() { swaggerContent := `swagger: "2.0" paths: @@ -4950,114 +4741,6 @@ definitions: }) }) - Convey("Given a swagger doc that exposes a resource with not valid multi region configuration (x-terraform-resource-regions-serviceProviderName is missing", t, func() { - var swaggerJSON = ` -{ - "swagger":"2.0", - "x-terraform-resource-regions-someOtherServiceProvider": "rst, dub", - "paths":{ - "/v1/cdns":{ - "post":{ - "x-terraform-resource-host": "some.api.${serviceProviderName}.domain.com", - "summary":"Create cdn", - "parameters":[ - { - "in":"body", - "name":"body", - "description":"Created CDN", - "schema":{ - "$ref":"#/definitions/ContentDeliveryNetwork" - } - } - ] - } - }, - "/v1/cdns/{id}":{ - "get":{ - "summary":"Get cdn by id" - }, - "put":{ - "summary":"Updated cdn" - }, - "delete":{ - "summary":"Delete cdn" - } - } - }, - "definitions":{ - "ContentDeliveryNetwork":{ - "type":"object", - "properties":{ - "id":{ - "type":"string" - } - } - } - } -}` - a := initAPISpecAnalyser(swaggerJSON) - Convey("When GetTerraformCompliantResources method is called", func() { - r, err := a.GetTerraformCompliantResources() - Convey("Then the value returned should be empty", func() { - So(err, ShouldBeNil) - So(r, ShouldBeEmpty) - }) - }) - }) - Convey("Given a swagger doc that exposes a resource with a multi region configuration but missing region", t, func() { - var swaggerJSON = ` -{ - "swagger":"2.0", - "x-terraform-resource-regions-serviceProviderName": "", - "paths":{ - "/v1/cdns":{ - "post":{ - "x-terraform-resource-host": "some.api.${serviceProviderName}.domain.com", - "summary":"Create cdn", - "parameters":[ - { - "in":"body", - "name":"body", - "description":"Created CDN", - "schema":{ - "$ref":"#/definitions/ContentDeliveryNetwork" - } - } - ] - } - }, - "/v1/cdns/{id}":{ - "get":{ - "summary":"Get cdn by id" - }, - "put":{ - "summary":"Updated cdn" - }, - "delete":{ - "summary":"Delete cdn" - } - } - }, - "definitions":{ - "ContentDeliveryNetwork":{ - "type":"object", - "properties":{ - "id":{ - "type":"string" - } - } - } - } -}` - a := initAPISpecAnalyser(swaggerJSON) - Convey("When GetTerraformCompliantResources method is called", func() { - r, err := a.GetTerraformCompliantResources() - Convey("Then the value returned should be empty", func() { - So(err, ShouldBeNil) - So(r, ShouldBeEmpty) - }) - }) - }) } func assertPropertyExists(properties SpecSchemaDefinitionProperties, name string) (bool, int) { diff --git a/tests/integration/resource_monitors_test.go b/tests/integration/resource_monitors_test.go index 230e7ee23..9725682f2 100644 --- a/tests/integration/resource_monitors_test.go +++ b/tests/integration/resource_monitors_test.go @@ -8,53 +8,9 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) -const resourceNameMonitor = "monitors_v1" - -var regionRst1 = "rst1" -var regionDub1 = "dub1" - -var openAPIResourceNameMonitorRst = fmt.Sprintf("%s_%s_%s", providerName, resourceNameMonitor, regionRst1) -var openAPIResourceNameMonitorDub = fmt.Sprintf("%s_%s_%s", providerName, resourceNameMonitor, regionDub1) var openAPIResourceInstanceNameMonitor = "my_monitor" var testCreateConfigMonitor string -var testCreateConfigMonitorMultiRegionProvider string - -func init() { - testCreateConfigMonitor = populateTemplateConfigurationMonitor() -} - -func TestAccMonitor_CreateRst1(t *testing.T) { - expectedValidationError, _ := regexp.Compile(".*request POST https://some.api.rst1.nonexistingrandomdomain.io/v1/monitors HTTP/1.1 failed. Response Error: 'Post \"https://some.api.rst1.nonexistingrandomdomain.io/v1/monitors\": dial tcp: lookup some.api.rst1.nonexistingrandomdomain.io.*") - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ProviderFactories: testAccProviders, - CheckDestroy: nil, - Steps: []resource.TestStep{ - { - Config: testCreateConfigMonitor, - Check: resource.ComposeTestCheckFunc(), - ExpectError: expectedValidationError, - }, - }, - }) -} - -func TestAccMonitor_CreateDub1(t *testing.T) { - expectedValidationError, _ := regexp.Compile(".*request POST https://some.api.dub1.nonexistingrandomdomain.io/v1/monitors HTTP/1.1 failed. Response Error: 'Post \"https://some.api.dub1.nonexistingrandomdomain.io/v1/monitors\": dial tcp: lookup some.api.dub1.nonexistingrandomdomain.io.*") - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ProviderFactories: testAccProviders, - CheckDestroy: nil, - Steps: []resource.TestStep{ - { - Config: testCreateConfigMonitor, - Check: resource.ComposeTestCheckFunc(), - ExpectError: expectedValidationError, - }, - }, - }) -} func TestAccMonitor_MultiRegion_CreateRst1(t *testing.T) { testCreateConfigMonitor = populateTemplateConfigurationMonitorServiceProvider("rst1") @@ -116,21 +72,6 @@ resource "openapi_multiregionmonitors_v1" "%s" { }) } -func populateTemplateConfigurationMonitor() string { - return fmt.Sprintf(`provider "%s" { - apikey_auth = "apiKeyValue" - x_request_id = "some value..." -} - -resource "%s" "%s" { - name = "someName" -} - -resource "%s" "%s" { - name = "someName" -}`, providerName, openAPIResourceNameMonitorRst, openAPIResourceInstanceNameMonitor, openAPIResourceNameMonitorDub, openAPIResourceInstanceNameMonitor) -} - func populateTemplateConfigurationMonitorServiceProvider(region string) string { return fmt.Sprintf(`provider "%s" { apikey_auth = "apiKeyValue"