Merge pull request #56 from dikhan/feature/resource-timout-extension

Feature/resource timout extension
dikhan · Sep 11, 2018 · bb41346 · bb41346
2 parents f7e6ccc + 53865f5
commit bb41346
Show file tree

Hide file tree

Showing 5 changed files with 240 additions and 11 deletions.
diff --git a/docs/how_to.md b/docs/how_to.md
@@ -262,11 +262,12 @@ The following extensions can be used in path operations. Read the according exte
 Extension Name | Type | Description
 ---|:---:|---
 [x-terraform-exclude-resource](#xTerraformExcludeResource) | bool | Only available in resource root's POST operation. Defines whether a given terraform compliant resource should be exposed to the OpenAPI Terraform provider or ignored.
+[x-terraform-resource-timeout](#xTerraformResourceTimeout) | string | Only available in operation level. Defines the timeout for a given operation. This value overrides the default timeout operation value which is 10 minutes.
 [x-terraform-header](#xTerraformHeader) | string | Only available in operation level parameters at the moment. Defines that he given header should be passed as part of the request.
-[x-terraform-resource-poll-enabled](#xTerraformResourcePollEnabled) | bool | Only supported in operation response (202). Defines that if the API responds with the given HTTP Status code (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-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)
 
 ###### <a name="xTerraformExcludeResource">x-terraform-exclude-resource</a>
- 
+
 Service providers might not want to expose certain resources to Terraform (e,g: admin resources). This can be achieved 
 by adding the following swagger extension to the resource root POST operation (in the example below ```/v1/resource:```):
 
@@ -289,6 +290,32 @@ this extension. If the extension is not present or has value 'false' then the re
 *Note: This extension is only interpreted and handled in resource root POST operations (e,g: /v1/resource) in the
 above example*
 
+###### <a name="xTerraformResourceTimeout">x-terraform-resource-timeout</a>
+
+This extension allows service providers to override the default timeout value for CRUD operations with a different value.
+
+The value must comply with the duration type format. A duration string is a sequence of decimal positive numbers (negative numbers are not allowed), 
+each with optional fraction and a unit suffix, such as "300s", "20.5m", "1.5h" or "2h45m".
+
+Valid time units are "s", "m", "h".
+
+````
+paths:
+  /v1/resource:
+    post:
+      ...
+      x-terraform-resource-timeout: "15m" # this means the max timeout for the post operation to finish is 15 minutes. This overrides the default timeout per operation which is 10 minutes
+      ...
+  /v1/resource/{id}:
+    get: # will have default value of 10 minutes as the 'x-terraform-resource-timeout' is not present for this operation
+      ...
+    delete:
+      x-terraform-resource-timeout: "20m" # this means the max timeout for the delete operation to finish is 20 minutes. This overrides the default timeout per operation which is 10 minutes
+      ...               
+````
+
+*Note: This extension is only supported at the operation level*
+
 ###### <a name="xTerraformHeader">x-terraform-header</a>  
 
 Certain operations may specify other type of parameters besides a 'body' type parameter which defines the payload expected 
@@ -400,7 +427,7 @@ definitions:
           - deleted          
 ````
 
-*Note: This extension is only supported at the response level.*
+*Note: This extension is only supported at the operation's response level.*
 
 #### <a name="swaggerDefinitions">Definitions</a>
 

diff --git a/examples/swaggercodegen/api/resources/swagger.yaml b/examples/swaggercodegen/api/resources/swagger.yaml
@@ -165,6 +165,7 @@ paths:
         required: true
         schema:
           $ref: "#/definitions/LBV1"
+      x-terraform-resource-timeout: "30s"
       responses:
         202: # Accepted
           x-terraform-resource-poll-enabled: true # [type (bool)] - this flags the response as trully async. Some resources might be async too but may require manual intervention from operators to complete the creation workflow. This flag will be used by the OpenAPI Service provider to detect whether the polling mechanism should be used or not. The flags below will only be applicable if this one is present with value 'true'
@@ -216,6 +217,7 @@ paths:
         required: true
         schema:
           $ref: "#/definitions/LBV1"
+#      x-terraform-resource-timeout: "30s" If a given operation does not have the 'x-terraform-resource-timeout' extension; the resource operation timeout will default to 10m (10 minutes)
       responses:
         202: # Accepted
           x-terraform-resource-poll-enabled: true

diff --git a/openapi/resource_factory.go b/openapi/resource_factory.go
@@ -33,15 +33,44 @@ func (r resourceFactory) createSchemaResource() (*schema.Resource, error) {
 	if err != nil {
 		return nil, err
 	}
+	timeouts, err := r.createSchemaResourceTimeout()
+	if err != nil {
+		return nil, err
+	}
 	return &schema.Resource{
-		Schema: s,
-		Create: r.create,
-		Read:   r.read,
-		Delete: r.delete,
-		Update: r.update,
-		Timeouts: &schema.ResourceTimeout{
-			Default: &defaultTimeout,
-		},
+		Schema:   s,
+		Create:   r.create,
+		Read:     r.read,
+		Delete:   r.delete,
+		Update:   r.update,
+		Timeouts: timeouts,
+	}, nil
+}
+
+func (r resourceFactory) createSchemaResourceTimeout() (*schema.ResourceTimeout, error) {
+	var postTimeout *time.Duration
+	var getTimeout *time.Duration
+	var putTimeout *time.Duration
+	var deleteTimeout *time.Duration
+	var err error
+	if postTimeout, err = r.resourceInfo.getResourceTimeout(r.resourceInfo.createPathInfo.Post); err != nil {
+		return nil, err
+	}
+	if getTimeout, err = r.resourceInfo.getResourceTimeout(r.resourceInfo.pathInfo.Get); err != nil {
+		return nil, err
+	}
+	if putTimeout, err = r.resourceInfo.getResourceTimeout(r.resourceInfo.pathInfo.Put); err != nil {
+		return nil, err
+	}
+	if deleteTimeout, err = r.resourceInfo.getResourceTimeout(r.resourceInfo.pathInfo.Delete); err != nil {
+		return nil, err
+	}
+	return &schema.ResourceTimeout{
+		Create:  postTimeout,
+		Read:    getTimeout,
+		Update:  putTimeout,
+		Delete:  deleteTimeout,
+		Default: &defaultTimeout,
 	}, nil
 }
 

diff --git a/openapi/resource_info.go b/openapi/resource_info.go
@@ -9,6 +9,8 @@ import (
 	"github.com/dikhan/terraform-provider-openapi/openapi/terraformutils"
 	"github.com/go-openapi/spec"
 	"github.com/hashicorp/terraform/helper/schema"
+	"regexp"
+	"time"
 )
 
 // Definition level extensions
@@ -24,6 +26,7 @@ const extTfExcludeResource = "x-terraform-exclude-resource"
 const extTfResourcePollEnabled = "x-terraform-resource-poll-enabled"
 const extTfResourcePollTargetStatuses = "x-terraform-resource-poll-completed-statuses"
 const extTfResourcePollPendingStatuses = "x-terraform-resource-poll-pending-statuses"
+const extTfResourceTimeout = "x-terraform-resource-timeout"
 
 const idDefaultPropertyName = "id"
 const statusDefaultPropertyName = "status"
@@ -325,6 +328,32 @@ func (r resourceInfo) getPollingStatuses(response spec.Response, extension strin
 	return statuses, nil
 }
 
+func (r resourceInfo) getResourceTimeout(operation *spec.Operation) (*time.Duration, error) {
+	if operation == nil {
+		return nil, nil
+	}
+	return r.getTimeDuration(operation.Extensions, extTfResourceTimeout)
+}
+
+func (r resourceInfo) getTimeDuration(extensions spec.Extensions, extension string) (*time.Duration, error) {
+	if value, exists := extensions.GetString(extension); exists {
+		regex, err := regexp.Compile("^\\d+(\\.\\d+)?[smh]{1}$")
+		if err != nil {
+			return nil, err
+		}
+		if !regex.Match([]byte(value)) {
+			return nil, fmt.Errorf("invalid duration value: '%s'. The value must be a sequence of decimal numbers each with optional fraction and a unit suffix (negative durations are not allowed). The value must be formatted either in seconds (s), minutes (m) or hours (h)", value)
+		}
+		return r.getDuration(value)
+	}
+	return nil, nil
+}
+
+func (r resourceInfo) getDuration(t string) (*time.Duration, error) {
+	duration, err := time.ParseDuration(t)
+	return &duration, err
+}
+
 func (r resourceInfo) isIDProperty(propertyName string) bool {
 	return r.propertyNameMatchesDefaultName(propertyName, idDefaultPropertyName)
 }

diff --git a/openapi/resource_info_test.go b/openapi/resource_info_test.go
@@ -9,6 +9,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestGetResourceURL(t *testing.T) {
@@ -1606,6 +1607,147 @@ func TestGetPollingStatuses(t *testing.T) {
 	})
 }
 
+func TestGetResourceTimeout(t *testing.T) {
+	Convey("Given a resourceInfo", t, func() {
+		r := resourceInfo{}
+		Convey(fmt.Sprintf("When getResourceTimeout method is called with an operation that has the extension '%s'", extTfResourceTimeout), func() {
+			expectedTimeout := "30s"
+			extensions := spec.Extensions{}
+			extensions.Add(extTfResourceTimeout, expectedTimeout)
+			post := &spec.Operation{
+				VendorExtensible: spec.VendorExtensible{
+					Extensions: extensions,
+				},
+			}
+			duration, err := r.getResourceTimeout(post)
+			Convey("Then the error returned should be nil", func() {
+				So(err, ShouldBeNil)
+			})
+			Convey("Then the duration returned should contain", func() {
+				So(*duration, ShouldEqual, time.Duration(30*time.Second))
+			})
+		})
+	})
+}
+
+func TestGetTimeDuration(t *testing.T) {
+	Convey("Given a resourceInfo", t, func() {
+		r := resourceInfo{}
+		Convey(fmt.Sprintf("When getTimeDuration method is called with a list of extensions that contains the extension passed in '%s' with value in seconds", extTfResourceTimeout), func() {
+			expectedTimeout := "30s"
+			extensions := spec.Extensions{}
+			extensions.Add(extTfResourceTimeout, expectedTimeout)
+			duration, err := r.getTimeDuration(extensions, extTfResourceTimeout)
+			Convey("Then the error returned should be nil", func() {
+				So(err, ShouldBeNil)
+			})
+			Convey("Then the duration returned should contain", func() {
+				So(*duration, ShouldEqual, time.Duration(30*time.Second))
+			})
+		})
+		Convey(fmt.Sprintf("When getTimeDuration method is called with a list of extensions that contains the extension passed in '%s' with value in minutes (using fractions)", extTfResourceTimeout), func() {
+			expectedTimeout := "20.5m"
+			extensions := spec.Extensions{}
+			extensions.Add(extTfResourceTimeout, expectedTimeout)
+			duration, err := r.getTimeDuration(extensions, extTfResourceTimeout)
+			Convey("Then the error returned should be nil", func() {
+				So(err, ShouldBeNil)
+			})
+			Convey("Then the duration returned should contain", func() {
+				So(*duration, ShouldEqual, time.Duration((20*time.Minute)+(30*time.Second)))
+			})
+		})
+		Convey(fmt.Sprintf("When getTimeDuration method is called with a list of extensions that contains the extension passed in '%s' with value in hours", extTfResourceTimeout), func() {
+			expectedTimeout := "1h"
+			extensions := spec.Extensions{}
+			extensions.Add(extTfResourceTimeout, expectedTimeout)
+			duration, err := r.getTimeDuration(extensions, extTfResourceTimeout)
+			Convey("Then the error returned should be nil", func() {
+				So(err, ShouldBeNil)
+			})
+			Convey("Then the duration returned should contain", func() {
+				So(*duration, ShouldEqual, time.Duration(1*time.Hour))
+			})
+		})
+		Convey(fmt.Sprintf("When getTimeDuration method is called with a list of extensions that DOES NOT contain the extension passed in '%s'", extTfResourceTimeout), func() {
+			expectedTimeout := "30s"
+			extensions := spec.Extensions{}
+			extensions.Add(extTfResourceTimeout, expectedTimeout)
+			duration, err := r.getTimeDuration(extensions, "nonExistingExtension")
+			Convey("Then the error returned should be nil", func() {
+				So(err, ShouldBeNil)
+			})
+			Convey("Then the duration returned should be nil", func() {
+				So(duration, ShouldBeNil)
+			})
+		})
+		Convey(fmt.Sprintf("When getTimeDuration method is called with a list of extensions that DOES contain the extension passed in '%s' BUT the value is an empty string", extTfResourceTimeout), func() {
+			extensions := spec.Extensions{}
+			extensions.Add(extTfResourceTimeout, "")
+			duration, err := r.getTimeDuration(extensions, extTfResourceTimeout)
+			Convey("Then the error returned should be nil", func() {
+				So(err, ShouldNotBeNil)
+			})
+			Convey("Then the duration returned should be nil", func() {
+				So(duration, ShouldBeNil)
+			})
+			Convey("And the error message should be", func() {
+				So(err.Error(), ShouldContainSubstring, "invalid duration value: ''. The value must be a sequence of decimal numbers each with optional fraction and a unit suffix (negative durations are not allowed). The value must be formatted either in seconds (s), minutes (m) or hours (h)")
+			})
+		})
+		Convey(fmt.Sprintf("When getTimeDuration method is called with a list of extensions that DOES contain the extension passed in '%s' BUT the value is a negative duration", extTfResourceTimeout), func() {
+			extensions := spec.Extensions{}
+			extensions.Add(extTfResourceTimeout, "-1.5h")
+			duration, err := r.getTimeDuration(extensions, extTfResourceTimeout)
+			Convey("Then the error returned should be nil", func() {
+				So(err, ShouldNotBeNil)
+			})
+			Convey("Then the duration returned should be nil", func() {
+				So(duration, ShouldBeNil)
+			})
+			Convey("And the error message should be", func() {
+				So(err.Error(), ShouldContainSubstring, "invalid duration value: '-1.5h'. The value must be a sequence of decimal numbers each with optional fraction and a unit suffix (negative durations are not allowed). The value must be formatted either in seconds (s), minutes (m) or hours (h)")
+			})
+		})
+		Convey(fmt.Sprintf("When getTimeDuration method is called with a list of extensions that DOES contain the extension passed in '%s' BUT the value is NOT supported (distinct than s,m and h)", extTfResourceTimeout), func() {
+			extensions := spec.Extensions{}
+			extensions.Add(extTfResourceTimeout, "300ms")
+			duration, err := r.getTimeDuration(extensions, extTfResourceTimeout)
+			Convey("Then the error returned should be nil", func() {
+				So(err, ShouldNotBeNil)
+			})
+			Convey("Then the duration returned should be nil", func() {
+				So(duration, ShouldBeNil)
+			})
+			Convey("And the error message should be", func() {
+				So(err.Error(), ShouldContainSubstring, "invalid duration value: '300ms'. The value must be a sequence of decimal numbers each with optional fraction and a unit suffix (negative durations are not allowed). The value must be formatted either in seconds (s), minutes (m) or hours (h)")
+			})
+		})
+	})
+}
+
+func TestGetDuration(t *testing.T) {
+	Convey("Given a resourceInfo", t, func() {
+		r := resourceInfo{}
+		Convey("When getDuration method is called a valid formatted time'", func() {
+			duration, err := r.getDuration("30s")
+			Convey("Then the error returned should be nil", func() {
+				So(err, ShouldBeNil)
+			})
+			Convey("Then the statuses returned should contain", func() {
+				fmt.Println(duration)
+				So(*duration, ShouldEqual, time.Duration(30*time.Second))
+			})
+		})
+		Convey("When getDuration method is called a invalid formatted time'", func() {
+			_, err := r.getDuration("some invalid formatted time")
+			Convey("Then the error returned should be nil", func() {
+				So(err, ShouldNotBeNil)
+			})
+		})
+	})
+}
+
 func TestShouldIgnoreResource(t *testing.T) {
 	Convey("Given a terraform compliant resource that has a POST operation containing the x-terraform-exclude-resource with value true", t, func() {
 		r := resourceInfo{