Repository: cloudflare/cloudflare-go
Pull request #1303 (2023-06-08 03:46:02)

tunnel: introduce custom `Duration` for time fields
All Tunnel timeout values are based on seconds however, there isn't a great way to do this in a flexible way with Go due to `time.Duration` [not having marshal/unmarshal support in Go 1](https://github.com/golang/go/issues/10275). Instead, we introduce a custom `TunnelDuration` value here and ensure it always converts durations into seconds without impacting other users of `time.Duration`.

This is similarly handled in [cloudflared too](https://github.com/cloudflare/cloudflared/blob/a3bcf25faeb02c8d8e0ef2d758cd8ed6aaba852a/config/configuration.go#L426).

jacobbednarz: cc @obezuk 
github-actions[bot]: changelog detected :white_check_mark:
codecov-commenter: ## [Codecov](https://app.codecov.io/gh/cloudflare/cloudflare-go/pull/1303?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudflare) Report
> Merging [#1303](https://app.codecov.io/gh/cloudflare/cloudflare-go/pull/1303?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudflare) (81cb9bc) into [master](https://app.codecov.io/gh/cloudflare/cloudflare-go/commit/5584c5bcdc437092677eb5398eeabcf28ca95a9d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudflare) (5584c5b) will **increase** coverage by `0.00%`.
> The diff coverage is `66.66%`.

```diff
@@           Coverage Diff           @@
##           master    #1303   +/-   ##
=======================================
  Coverage   49.07%   49.08%           
=======================================
  Files         134      134           
  Lines       13090    13099    +9     
=======================================
+ Hits         6424     6429    +5     
- Misses       5162     5165    +3     
- Partials     1504     1505    +1     
```


| [Impacted Files](https://app.codecov.io/gh/cloudflare/cloudflare-go/pull/1303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudflare) | Coverage Δ | |
|---|---|---|
| [access\_application.go](https://app.codecov.io/gh/cloudflare/cloudflare-go/pull/1303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudflare#diff-YWNjZXNzX2FwcGxpY2F0aW9uLmdv) | `76.52% <ø> (ø)` | |
| [load\_balancing.go](https://app.codecov.io/gh/cloudflare/cloudflare-go/pull/1303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudflare#diff-bG9hZF9iYWxhbmNpbmcuZ28=) | `59.40% <ø> (ø)` | |
| [stream.go](https://app.codecov.io/gh/cloudflare/cloudflare-go/pull/1303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudflare#diff-c3RyZWFtLmdv) | `64.91% <ø> (ø)` | |
| [tunnel.go](https://app.codecov.io/gh/cloudflare/cloudflare-go/pull/1303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudflare#diff-dHVubmVsLmdv) | `42.41% <66.66%> (+1.01%)` | :arrow_up: |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/cloudflare/cloudflare-go/pull/1303/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudflare)



Patch:
diff --git a/.changelog/1303.txt b/.changelog/1303.txt
new file mode 100644
index 000000000..50a9b97a3
--- /dev/null
+++ b/.changelog/1303.txt
@@ -0,0 +1,3 @@
+```release-note:breaking-change
+tunnel: swap `ConnectTimeout`, `TLSTimeout`, `TCPKeepAlive` and `KeepAliveTimeout` to `TunnelDuration` instead of `time.Duration`
+```
diff --git a/tunnel.go b/tunnel.go
index 30fa97896..92e91b991 100644
--- a/tunnel.go
+++ b/tunnel.go
@@ -6,9 +6,34 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"strconv"
 	"time"
 )
 
+// A TunnelDuration is a Duration that has custom serialization for JSON.
+// JSON in Javascript assumes that int fields are 32 bits and Duration fields
+// are deserialized assuming that numbers are in nanoseconds, which in 32bit
+// integers limits to just 2 seconds. This type assumes that when
+// serializing/deserializing from JSON, that the number is in seconds, while it
+// maintains the YAML serde assumptions.
+type TunnelDuration struct {
+	time.Duration
+}
+
+func (s TunnelDuration) MarshalJSON() ([]byte, error) {
+	return json.Marshal(s.Duration.Seconds())
+}
+
+func (s *TunnelDuration) UnmarshalJSON(data []byte) error {
+	seconds, err := strconv.ParseInt(string(data), 10, 64)
+	if err != nil {
+		return err
+	}
+
+	s.Duration = time.Duration(seconds * int64(time.Second))
+	return nil
+}
+
 // ErrMissingTunnelID is for when a required tunnel ID is missing from the
 // parameters.
 var ErrMissingTunnelID = errors.New("required missing tunnel ID")
@@ -118,17 +143,17 @@ type UnvalidatedIngressRule struct {
 // config.
 type OriginRequestConfig struct {
 	// HTTP proxy timeout for establishing a new connection
-	ConnectTimeout *time.Duration `json:"connectTimeout,omitempty"`
+	ConnectTimeout *TunnelDuration `json:"connectTimeout,omitempty"`
 	// HTTP proxy timeout for completing a TLS handshake
-	TLSTimeout *time.Duration `json:"tlsTimeout,omitempty"`
+	TLSTimeout *TunnelDuration `json:"tlsTimeout,omitempty"`
 	// HTTP proxy TCP keepalive duration
-	TCPKeepAlive *time.Duration `json:"tcpKeepAlive,omitempty"`
+	TCPKeepAlive *TunnelDuration `json:"tcpKeepAlive,omitempty"`
 	// HTTP proxy should disable "happy eyeballs" for IPv4/v6 fallback
 	NoHappyEyeballs *bool `json:"noHappyEyeballs,omitempty"`
 	// HTTP proxy maximum keepalive connection pool size
 	KeepAliveConnections *int `json:"keepAliveConnections,omitempty"`
 	// HTTP proxy timeout for closing an idle connection
-	KeepAliveTimeout *time.Duration `json:"keepAliveTimeout,omitempty"`
+	KeepAliveTimeout *TunnelDuration `json:"keepAliveTimeout,omitempty"`
 	// Sets the HTTP Host header for the local webserver.
 	HTTPHostHeader *string `json:"httpHostHeader,omitempty"`
 	// Hostname on the origin server certificate.


Test patch:
diff --git a/tunnel_test.go b/tunnel_test.go
index cacc12fb6..da912ccf4 100644
--- a/tunnel_test.go
+++ b/tunnel_test.go
@@ -189,6 +189,7 @@ func TestUpdateTunnelConfiguration(t *testing.T) {
 		fmt.Fprint(w, loadFixture("tunnel", "configuration"))
 	}
 
+	timeout, _ := time.ParseDuration("10s")
 	mux.HandleFunc(fmt.Sprintf("/accounts/%s/cfd_tunnel/%s/configurations", testAccountID, testTunnelID), handler)
 	want := TunnelConfigurationResult{
 		TunnelID: testTunnelID,
@@ -210,7 +211,7 @@ func TestUpdateTunnelConfiguration(t *testing.T) {
 				Enabled: true,
 			},
 			OriginRequest: OriginRequestConfig{
-				ConnectTimeout: DurationPtr(10),
+				ConnectTimeout: &TunnelDuration{timeout},
 			},
 		}}
 
@@ -233,7 +234,7 @@ func TestUpdateTunnelConfiguration(t *testing.T) {
 				Enabled: true,
 			},
 			OriginRequest: OriginRequestConfig{
-				ConnectTimeout: DurationPtr(10 * time.Second),
+				ConnectTimeout: &TunnelDuration{10},
 			},
 		},
 	})
@@ -254,6 +255,7 @@ func TestGetTunnelConfiguration(t *testing.T) {
 		fmt.Fprint(w, loadFixture("tunnel", "configuration"))
 	}
 
+	timeout, _ := time.ParseDuration("10s")
 	mux.HandleFunc(fmt.Sprintf("/accounts/%s/cfd_tunnel/%s/configurations", testAccountID, testTunnelID), handler)
 	want := TunnelConfigurationResult{
 		TunnelID: testTunnelID,
@@ -275,7 +277,7 @@ func TestGetTunnelConfiguration(t *testing.T) {
 				Enabled: true,
 			},
 			OriginRequest: OriginRequestConfig{
-				ConnectTimeout: DurationPtr(10),
+				ConnectTimeout: &TunnelDuration{timeout},
 			},
 		}}