Update type for timeouts from time.Duration to metav1.Duration

- This allows the values of these timeouts to be set as durations via the CRD allowing timeouts to be set as '5s' instead of an integer that time.Duration is resolved as.
hashicorp · Jun 8, 2021 · 8bcaee1 · 8bcaee1
1 parent 46cdc34
commit 8bcaee1
Show file tree

Hide file tree

Showing 8 changed files with 27 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,11 @@ IMPROVEMENTS:
 * Connect: skip service registration when a service with the same name but in a different Kubernetes namespace is found
   and Consul namespaces are not enabled. [[GH-527](https://github.com/hashicorp/consul-k8s/pull/527)]
 
+BUG FIXES:
+* CRDs: Update the type of connectTimeout and TTL in ServiceResolver and ServiceRouter from time.Duration to metav1.Duration.
+  This allows a user to set these values as a duration string on the resource. Existing resources that had set a specific integer
+  duration will continue to function with a duration with 'n' nanoseconds, 'n' being the set value.
+
 ## 0.26.0-beta3 (May 27, 2021)
 
 IMPROVEMENTS:

diff --git a/api/v1alpha1/serviceresolver_types.go b/api/v1alpha1/serviceresolver_types.go
@@ -2,7 +2,6 @@ package v1alpha1
 
 import (
 	"encoding/json"
-	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -68,7 +67,7 @@ type ServiceResolverSpec struct {
 	Failover ServiceResolverFailoverMap `json:"failover,omitempty"`
 	// ConnectTimeout is the timeout for establishing new network connections
 	// to this service.
-	ConnectTimeout time.Duration `json:"connectTimeout,omitempty"`
+	ConnectTimeout metav1.Duration `json:"connectTimeout,omitempty"`
 	// LoadBalancer determines the load balancing policy and configuration for services
 	// issuing requests to this upstream service.
 	LoadBalancer *LoadBalancer `json:"loadBalancer,omitempty"`
@@ -181,7 +180,7 @@ type CookieConfig struct {
 	Session bool `json:"session,omitempty"`
 
 	// TTL is the ttl for generated cookies. Cannot be specified for session cookies.
-	TTL time.Duration `json:"ttl,omitempty"`
+	TTL metav1.Duration `json:"ttl,omitempty"`
 
 	// Path is the path to set for the cookie.
 	Path string `json:"path,omitempty"`
@@ -270,7 +269,7 @@ func (in *ServiceResolver) ToConsul(datacenter string) capi.ConfigEntry {
 		Subsets:        in.Spec.Subsets.toConsul(),
 		Redirect:       in.Spec.Redirect.toConsul(),
 		Failover:       in.Spec.Failover.toConsul(),
-		ConnectTimeout: in.Spec.ConnectTimeout,
+		ConnectTimeout: in.Spec.ConnectTimeout.Duration,
 		LoadBalancer:   in.Spec.LoadBalancer.toConsul(),
 		Meta:           meta(datacenter),
 	}
@@ -419,7 +418,7 @@ func (in *CookieConfig) toConsul() *capi.CookieConfig {
 	}
 	return &capi.CookieConfig{
 		Session: in.Session,
-		TTL:     in.TTL,
+		TTL:     in.TTL.Duration,
 		Path:    in.Path,
 	}
 }
@@ -429,7 +428,7 @@ func (in *CookieConfig) validate(path *field.Path) *field.Error {
 		return nil
 	}
 
-	if in.Session && in.TTL > 0 {
+	if in.Session && in.TTL.Duration > 0 {
 		asJSON, _ := json.Marshal(in)
 		return field.Invalid(path, string(asJSON), "cannot set both session and ttl")
 	}

diff --git a/api/v1alpha1/serviceresolver_types_test.go b/api/v1alpha1/serviceresolver_types_test.go
@@ -74,7 +74,7 @@ func TestServiceResolver_MatchesConsul(t *testing.T) {
 							Datacenters:   []string{"failover2_dc1", "failover2_dc2"},
 						},
 					},
-					ConnectTimeout: 1 * time.Second,
+					ConnectTimeout: metav1.Duration{Duration: 1 * time.Second},
 					LoadBalancer: &LoadBalancer{
 						Policy: "policy",
 						RingHashConfig: &RingHashConfig{
@@ -90,7 +90,7 @@ func TestServiceResolver_MatchesConsul(t *testing.T) {
 								FieldValue: "value",
 								CookieConfig: &CookieConfig{
 									Session: true,
-									TTL:     1,
+									TTL:     metav1.Duration{Duration: 1},
 									Path:    "path",
 								},
 								SourceIP: true,
@@ -243,7 +243,7 @@ func TestServiceResolver_ToConsul(t *testing.T) {
 							Datacenters:   []string{"failover2_dc1", "failover2_dc2"},
 						},
 					},
-					ConnectTimeout: 1 * time.Second,
+					ConnectTimeout: metav1.Duration{Duration: 1 * time.Second},
 					LoadBalancer: &LoadBalancer{
 						Policy: "policy",
 						RingHashConfig: &RingHashConfig{
@@ -259,7 +259,7 @@ func TestServiceResolver_ToConsul(t *testing.T) {
 								FieldValue: "value",
 								CookieConfig: &CookieConfig{
 									Session: true,
-									TTL:     1,
+									TTL:     metav1.Duration{Duration: 1},
 									Path:    "path",
 								},
 								SourceIP: true,
@@ -634,7 +634,7 @@ func TestServiceResolver_Validate(t *testing.T) {
 								FieldValue: "cookiename",
 								CookieConfig: &CookieConfig{
 									Session: true,
-									TTL:     100,
+									TTL:     metav1.Duration{Duration: 100},
 								},
 							},
 						},
@@ -643,7 +643,7 @@ func TestServiceResolver_Validate(t *testing.T) {
 			},
 			namespacesEnabled: false,
 			expectedErrMsgs: []string{
-				`serviceresolver.consul.hashicorp.com "foo" is invalid: spec.loadBalancer.hashPolicies[0].cookieConfig: Invalid value: "{\"session\":true,\"ttl\":100}": cannot set both session and ttl`,
+				`serviceresolver.consul.hashicorp.com "foo" is invalid: spec.loadBalancer.hashPolicies[0].cookieConfig: Invalid value: "{\"session\":true,\"ttl\":\"100ns\"}": cannot set both session and ttl`,
 			},
 		},
 		"namespaces disabled: redirect namespace specified": {

diff --git a/api/v1alpha1/servicerouter_types.go b/api/v1alpha1/servicerouter_types.go
@@ -2,7 +2,6 @@ package v1alpha1
 
 import (
 	"encoding/json"
-	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -135,7 +134,7 @@ type ServiceRouteDestination struct {
 	PrefixRewrite string `json:"prefixRewrite,omitempty"`
 	// RequestTimeout is the total amount of time permitted for the entire
 	// downstream request (and retries) to be processed.
-	RequestTimeout time.Duration `json:"requestTimeout,omitempty"`
+	RequestTimeout metav1.Duration `json:"requestTimeout,omitempty"`
 	// NumRetries is the number of times to retry the request when a retryable result occurs
 	NumRetries uint32 `json:"numRetries,omitempty"`
 	// RetryOnConnectFailure allows for connection failure errors to trigger a retry.
@@ -326,7 +325,7 @@ func (in *ServiceRouteDestination) toConsul() *capi.ServiceRouteDestination {
 		ServiceSubset:         in.ServiceSubset,
 		Namespace:             in.Namespace,
 		PrefixRewrite:         in.PrefixRewrite,
-		RequestTimeout:        in.RequestTimeout,
+		RequestTimeout:        in.RequestTimeout.Duration,
 		NumRetries:            in.NumRetries,
 		RetryOnConnectFailure: in.RetryOnConnectFailure,
 		RetryOnStatusCodes:    in.RetryOnStatusCodes,

diff --git a/api/v1alpha1/servicerouter_types_test.go b/api/v1alpha1/servicerouter_types_test.go
@@ -79,7 +79,7 @@ func TestServiceRouter_MatchesConsul(t *testing.T) {
 								ServiceSubset:         "serviceSubset",
 								Namespace:             "namespace",
 								PrefixRewrite:         "prefixRewrite",
-								RequestTimeout:        1 * time.Second,
+								RequestTimeout:        metav1.Duration{Duration: 1 * time.Second},
 								NumRetries:            1,
 								RetryOnConnectFailure: true,
 								RetryOnStatusCodes:    []uint32{500, 400},
@@ -220,7 +220,7 @@ func TestServiceRouter_ToConsul(t *testing.T) {
 								ServiceSubset:         "serviceSubset",
 								Namespace:             "namespace",
 								PrefixRewrite:         "prefixRewrite",
-								RequestTimeout:        1 * time.Second,
+								RequestTimeout:        metav1.Duration{Duration: 1 * time.Second},
 								NumRetries:            1,
 								RetryOnConnectFailure: true,
 								RetryOnStatusCodes:    []uint32{500, 400},
@@ -543,7 +543,7 @@ func TestServiceRouter_Validate(t *testing.T) {
 			},
 			namespacesEnabled: false,
 			expectedErrMsgs: []string{
-				`servicerouter.consul.hashicorp.com "foo" is invalid: spec.routes[0]: Invalid value: "{\"match\":{\"http\":{}},\"destination\":{\"prefixRewrite\":\"prefixRewrite\"}}": destination.prefixRewrite requires that either match.http.pathPrefix or match.http.pathExact be configured on this route`,
+				`servicerouter.consul.hashicorp.com "foo" is invalid: spec.routes[0]: Invalid value: "{\"match\":{\"http\":{}},\"destination\":{\"prefixRewrite\":\"prefixRewrite\",\"requestTimeout\":\"0s\"}}": destination.prefixRewrite requires that either match.http.pathPrefix or match.http.pathExact be configured on this route`,
 			},
 		},
 		"namespaces disabled: single destination namespace specified": {

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/consul.hashicorp.com_serviceresolvers.yaml b/config/crd/bases/consul.hashicorp.com_serviceresolvers.yaml
@@ -47,8 +47,7 @@ spec:
             properties:
               connectTimeout:
                 description: ConnectTimeout is the timeout for establishing new network connections to this service.
-                format: int64
-                type: integer
+                type: string
               defaultSubset:
                 description: DefaultSubset is the subset to use when no explicit subset is requested. If empty the unnamed subset is used.
                 type: string
@@ -90,8 +89,7 @@ spec:
                               type: boolean
                             ttl:
                               description: TTL is the ttl for generated cookies. Cannot be specified for session cookies.
-                              format: int64
-                              type: integer
+                              type: string
                           type: object
                         field:
                           description: Field is the attribute type to hash on. Must be one of "header", "cookie", or "query_parameter". Cannot be specified along with sourceIP.

diff --git a/config/crd/bases/consul.hashicorp.com_servicerouters.yaml b/config/crd/bases/consul.hashicorp.com_servicerouters.yaml
@@ -64,8 +64,7 @@ spec:
                           type: string
                         requestTimeout:
                           description: RequestTimeout is the total amount of time permitted for the entire downstream request (and retries) to be processed.
-                          format: int64
-                          type: integer
+                          type: string
                         retryOnConnectFailure:
                           description: RetryOnConnectFailure allows for connection failure errors to trigger a retry.
                           type: boolean