Add timeout and retry to framework version of provider (#151)

* Adding optional request_timeout and retry attributes (#149)

Co-authored-by: Vincent Chenal <vincent.chenal@protonmail.com>

* Fixing ExpectError in tests (#149)

* Updates following code review (#149)

---------

Co-authored-by: Vincent Chenal <vincent.chenal@protonmail.com>

Author: bendbennett

.changes/unreleased/ENHANCEMENTS-20230209-165728.yaml

@@ -0,0 +1,5 @@
+kind: ENHANCEMENTS
+body: 'data-source/http: Added `retry` with nested `attempts`, `max_delay_ms` and `min_delay_ms`'
+time: 2023-02-09T16:57:28.046924Z
+custom:
+  Issue: "151"

.changes/unreleased/ENHANCEMENTS-20230209-165747.yaml

@@ -0,0 +1,5 @@
+kind: ENHANCEMENTS
+body: 'data-source/http: Added `request_timeout_ms`'
+time: 2023-02-09T16:57:47.790638Z
+custom:
+  Issue: "151"

docs/data-sources/http.md

@@ -12,6 +12,10 @@ description: |-
   mechanism to authenticate the remote server except for general verification of
   the server certificate's chain of trust. Data retrieved from servers not under
   your control should be treated as untrustworthy.
+  By default, there are no retries. Configuring the retry block will result in
+  retries if an error is returned by the client (e.g., connection errors) or if
+  a 5xx-range (except 501) status code is received. For further details see
+  go-retryablehttp https://pkg.go.dev/github.com/hashicorp/go-retryablehttp.
 ---
 
 # http (Data Source)
@@ -29,6 +33,11 @@ mechanism to authenticate the remote server except for general verification of
 the server certificate's chain of trust. Data retrieved from servers not under
 your control should be treated as untrustworthy.
 
+By default, there are no retries. Configuring the retry block will result in
+retries if an error is returned by the client (e.g., connection errors) or if 
+a 5xx-range (except 501) status code is received. For further details see 
+[go-retryablehttp](https://pkg.go.dev/github.com/hashicorp/go-retryablehttp).
+
 ## Example Usage
 
 ```terraform
@@ -148,11 +157,22 @@ resource "null_resource" "example" {
 - `method` (String) The HTTP Method for the request. Allowed methods are a subset of methods defined in [RFC7231](https://datatracker.ietf.org/doc/html/rfc7231#section-4.3) namely, `GET`, `HEAD`, and `POST`. `POST` support is only intended for read-only URLs, such as submitting a search.
 - `request_body` (String) The request body as a string.
 - `request_headers` (Map of String) A map of request header field names and values.
+- `request_timeout_ms` (Number) The request timeout in milliseconds.
+- `retry` (Block, Optional) Retry request configuration. By default there are no retries. Configuring this block will result in retries if an error is returned by the client (e.g., connection errors) or if a 5xx-range (except 501) status code is received. For further details see [go-retryablehttp](https://pkg.go.dev/github.com/hashicorp/go-retryablehttp). (see [below for nested schema](#nestedblock--retry))
 
 ### Read-Only
 
 - `body` (String, Deprecated) The response body returned as a string. **NOTE**: This is deprecated, use `response_body` instead.
 - `id` (String) The URL used for the request.
 - `response_body` (String) The response body returned as a string.
 - `response_headers` (Map of String) A map of response header field names and values. Duplicate headers are concatenated according to [RFC2616](https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2).
-- `status_code` (Number) The HTTP response status code.
+- `status_code` (Number) The HTTP response status code.
+
+<a id="nestedblock--retry"></a>
+### Nested Schema for `retry`
+
+Optional:
+
+- `attempts` (Number) The number of times the request is to be retried. For example, if 2 is specified, the request will be tried a maximum of 3 times.
+- `max_delay_ms` (Number) The maximum delay between retry requests in milliseconds.
+- `min_delay_ms` (Number) The minimum delay between retry requests in milliseconds.

go.mod

@@ -3,10 +3,13 @@ module github.com/terraform-providers/terraform-provider-http
 go 1.19
 
 require (
+	github.com/google/uuid v1.3.0
+	github.com/hashicorp/go-retryablehttp v0.7.1
 	github.com/hashicorp/terraform-plugin-docs v0.14.1
 	github.com/hashicorp/terraform-plugin-framework v1.2.0
 	github.com/hashicorp/terraform-plugin-framework-validators v0.10.0
 	github.com/hashicorp/terraform-plugin-go v0.15.0
+	github.com/hashicorp/terraform-plugin-log v0.8.0
 	github.com/hashicorp/terraform-plugin-testing v1.2.0
 	golang.org/x/net v0.9.0
 )
@@ -22,7 +25,6 @@ require (
 	github.com/fatih/color v1.13.0 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/go-cmp v0.5.9 // indirect
-	github.com/google/uuid v1.3.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-checkpoint v0.5.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
@@ -37,7 +39,6 @@ require (
 	github.com/hashicorp/logutils v1.0.0 // indirect
 	github.com/hashicorp/terraform-exec v0.18.1 // indirect
 	github.com/hashicorp/terraform-json v0.16.0 // indirect
-	github.com/hashicorp/terraform-plugin-log v0.8.0 // indirect
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.26.1 // indirect
 	github.com/hashicorp/terraform-registry-address v0.2.0 // indirect
 	github.com/hashicorp/terraform-svchost v0.0.1 // indirect

go.sum

@@ -65,17 +65,21 @@ github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brv
 github.com/hashicorp/go-checkpoint v0.5.0 h1:MFYpPZCnQqQTE18jFwSII6eUQrD/oxMFp3mlgcqk5mU=
 github.com/hashicorp/go-checkpoint v0.5.0/go.mod h1:7nfLNL10NsxqO4iWuW6tWW0HjZuDrwkBuEQsVcpCOgg=
 github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
+github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
 github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
 github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
 github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 h1:1/D3zfFHttUKaCaGKZ/dR2roBXv0vKbSCnssIldfQdI=
 github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320/go.mod h1:EiZBMaudVLy8fmjf9Npq1dq9RalhveqZG5w/yz3mHWs=
+github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
 github.com/hashicorp/go-hclog v1.5.0 h1:bI2ocEMgcVlz55Oj1xZNBsVi900c7II+fWDyV9o+13c=
 github.com/hashicorp/go-hclog v1.5.0/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M=
 github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
 github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
 github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
 github.com/hashicorp/go-plugin v1.4.9 h1:ESiK220/qE0aGxWdzKIvRH69iLiuN/PjoLTm69RoWtU=
 github.com/hashicorp/go-plugin v1.4.9/go.mod h1:viDMjcLJuDui6pXb8U4HVfb8AamCWhHGUjr2IrTF67s=
+github.com/hashicorp/go-retryablehttp v0.7.1 h1:sUiuQAnLlbvmExtFQs72iFW/HXeUn8Z1aJLQ4LJJbTQ=
+github.com/hashicorp/go-retryablehttp v0.7.1/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY=
 github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
 github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8=
 github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=

internal/provider/data_source_http.go

@@ -4,21 +4,27 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"io"
 	"mime"
 	"net/http"
 	"net/url"
 	"regexp"
 	"strings"
+	"time"
 
+	"github.com/hashicorp/go-retryablehttp"
+	"github.com/hashicorp/terraform-plugin-framework-validators/int64validator"
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"github.com/hashicorp/terraform-plugin-framework/datasource"
 	"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/tfsdk"
 	"github.com/hashicorp/terraform-plugin-framework/types"
+	"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"golang.org/x/net/http/httpproxy"
 )
 
@@ -52,7 +58,13 @@ regardless of the returned content type header.
 ~> **Important** Although ` + "`https`" + ` URLs can be used, there is currently no
 mechanism to authenticate the remote server except for general verification of
 the server certificate's chain of trust. Data retrieved from servers not under
-your control should be treated as untrustworthy.`,
+your control should be treated as untrustworthy.
+
+By default, there are no retries. Configuring the retry block will result in
+retries if an error is returned by the client (e.g., connection errors) or if 
+a 5xx-range (except 501) status code is received. For further details see 
+[go-retryablehttp](https://pkg.go.dev/github.com/hashicorp/go-retryablehttp).
+`,
 
 		Attributes: map[string]schema.Attribute{
 			"id": schema.StringAttribute{
@@ -90,6 +102,14 @@ your control should be treated as untrustworthy.`,
 				Optional:    true,
 			},
 
+			"request_timeout_ms": schema.Int64Attribute{
+				Description: "The request timeout in milliseconds.",
+				Optional:    true,
+				Validators: []validator.Int64{
+					int64validator.AtLeast(1),
+				},
+			},
+
 			"response_body": schema.StringAttribute{
 				Description: "The response body returned as a string.",
 				Computed:    true,
@@ -128,6 +148,38 @@ your control should be treated as untrustworthy.`,
 				Computed:    true,
 			},
 		},
+
+		Blocks: map[string]schema.Block{
+			"retry": schema.SingleNestedBlock{
+				Description: "Retry request configuration. By default there are no retries. Configuring this block will result in " +
+					"retries if an error is returned by the client (e.g., connection errors) or if a 5xx-range (except 501) status code is received. " +
+					"For further details see [go-retryablehttp](https://pkg.go.dev/github.com/hashicorp/go-retryablehttp).",
+				Attributes: map[string]schema.Attribute{
+					"attempts": schema.Int64Attribute{
+						Description: "The number of times the request is to be retried. For example, if 2 is specified, the request will be tried a maximum of 3 times.",
+						Optional:    true,
+						Validators: []validator.Int64{
+							int64validator.AtLeast(0),
+						},
+					},
+					"min_delay_ms": schema.Int64Attribute{
+						Description: "The minimum delay between retry requests in milliseconds.",
+						Optional:    true,
+						Validators: []validator.Int64{
+							int64validator.AtLeast(0),
+						},
+					},
+					"max_delay_ms": schema.Int64Attribute{
+						Description: "The maximum delay between retry requests in milliseconds.",
+						Optional:    true,
+						Validators: []validator.Int64{
+							int64validator.AtLeast(0),
+							int64validator.AtLeastSumOf(path.MatchRelative().AtParent().AtName("min_delay_ms")),
+						},
+					},
+				},
+			},
+		},
 	}
 }
 
@@ -195,11 +247,38 @@ func (d *httpDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
 		clonedTr.TLSClientConfig.RootCAs = caCertPool
 	}
 
-	client := &http.Client{
-		Transport: clonedTr,
+	var retry retryModel
+
+	if !model.Retry.IsNull() && !model.Retry.IsUnknown() {
+		diags = model.Retry.As(ctx, &retry, basetypes.ObjectAsOptions{})
+		resp.Diagnostics.Append(diags...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
+	}
+
+	retryClient := retryablehttp.NewClient()
+	retryClient.HTTPClient.Transport = clonedTr
+
+	var timeout time.Duration
+
+	if model.RequestTimeout.ValueInt64() > 0 {
+		timeout = time.Duration(model.RequestTimeout.ValueInt64()) * time.Millisecond
+		retryClient.HTTPClient.Timeout = timeout
 	}
 
-	request, err := http.NewRequestWithContext(ctx, method, requestURL, requestBody)
+	retryClient.Logger = levelledLogger{ctx}
+	retryClient.RetryMax = int(retry.Attempts.ValueInt64())
+
+	if !retry.MinDelay.IsNull() && !retry.MinDelay.IsUnknown() && retry.MinDelay.ValueInt64() >= 0 {
+		retryClient.RetryWaitMin = time.Duration(retry.MinDelay.ValueInt64()) * time.Millisecond
+	}
+
+	if !retry.MaxDelay.IsNull() && !retry.MaxDelay.IsUnknown() && retry.MaxDelay.ValueInt64() >= 0 {
+		retryClient.RetryWaitMax = time.Duration(retry.MaxDelay.ValueInt64()) * time.Millisecond
+	}
+
+	request, err := retryablehttp.NewRequestWithContext(ctx, method, requestURL, requestBody)
 	if err != nil {
 		resp.Diagnostics.AddError(
 			"Error creating request",
@@ -219,8 +298,25 @@ func (d *httpDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
 		request.Header.Set(name, header)
 	}
 
-	response, err := client.Do(request)
+	response, err := retryClient.Do(request)
 	if err != nil {
+		target := &url.Error{}
+		if errors.As(err, &target) {
+			if target.Timeout() {
+				detail := fmt.Sprintf("timeout error: %s", err)
+
+				if timeout > 0 {
+					detail = fmt.Sprintf("request exceeded the specified timeout: %s, err: %s", timeout.String(), err)
+				}
+
+				resp.Diagnostics.AddError(
+					"Error making request",
+					detail,
+				)
+				return
+			}
+		}
+
 		resp.Diagnostics.AddError(
 			"Error making request",
 			fmt.Sprintf("Error making request: %s", err),
@@ -251,8 +347,7 @@ func (d *httpDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
 
 	responseHeaders := make(map[string]string)
 	for k, v := range response.Header {
-		// Concatenate according to RFC2616
-		// cf. https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
+		// Concatenate according to RFC9110 https://www.rfc-editor.org/rfc/rfc9110.html#section-5.2
 		responseHeaders[k] = strings.Join(v, ", ")
 	}
 
@@ -304,10 +399,51 @@ type modelV0 struct {
 	Method          types.String `tfsdk:"method"`
 	RequestHeaders  types.Map    `tfsdk:"request_headers"`
 	RequestBody     types.String `tfsdk:"request_body"`
+	RequestTimeout  types.Int64  `tfsdk:"request_timeout_ms"`
+	Retry           types.Object `tfsdk:"retry"`
 	ResponseHeaders types.Map    `tfsdk:"response_headers"`
 	CaCertificate   types.String `tfsdk:"ca_cert_pem"`
 	Insecure        types.Bool   `tfsdk:"insecure"`
 	ResponseBody    types.String `tfsdk:"response_body"`
 	Body            types.String `tfsdk:"body"`
 	StatusCode      types.Int64  `tfsdk:"status_code"`
 }
+
+type retryModel struct {
+	Attempts types.Int64 `tfsdk:"attempts"`
+	MinDelay types.Int64 `tfsdk:"min_delay_ms"`
+	MaxDelay types.Int64 `tfsdk:"max_delay_ms"`
+}
+
+var _ retryablehttp.LeveledLogger = levelledLogger{}
+
+// levelledLogger is used to log messages from retryablehttp.Client to tflog.
+type levelledLogger struct {
+	ctx context.Context
+}
+
+func (l levelledLogger) Error(msg string, keysAndValues ...interface{}) {
+	tflog.Error(l.ctx, msg, l.additionalFields(keysAndValues))
+}
+
+func (l levelledLogger) Info(msg string, keysAndValues ...interface{}) {
+	tflog.Info(l.ctx, msg, l.additionalFields(keysAndValues))
+}
+
+func (l levelledLogger) Debug(msg string, keysAndValues ...interface{}) {
+	tflog.Debug(l.ctx, msg, l.additionalFields(keysAndValues))
+}
+
+func (l levelledLogger) Warn(msg string, keysAndValues ...interface{}) {
+	tflog.Warn(l.ctx, msg, l.additionalFields(keysAndValues))
+}
+
+func (l levelledLogger) additionalFields(keysAndValues []interface{}) map[string]interface{} {
+	additionalFields := make(map[string]interface{}, len(keysAndValues))
+
+	for i := 0; i+1 < len(keysAndValues); i += 2 {
+		additionalFields[fmt.Sprint(keysAndValues[i])] = keysAndValues[i+1]
+	}
+
+	return additionalFields
+}