Address per comments, added maxDuration check

pkg/utils/duration.go

@@ -37,7 +37,7 @@ func ParseDuration(s string) (*time.Duration, error) {
 
 		See https://gateway-api.sigs.k8s.io/geps/gep-2257/ for more details.
 	*/
-	if matched := re.MatchString(s); matched == false {
+	if !re.MatchString(s) {
 		return nil, errors.New("Invalid duration format")
 	}
 	parsedTime, err := time.ParseDuration(s)
@@ -46,11 +46,12 @@ func ParseDuration(s string) (*time.Duration, error) {
 	}
 
 	return &parsedTime, nil
-
 }
 
 var reSplit = regexp.MustCompile(`[0-9]{1,5}(h|ms|s|m)`)
 
+const maxDuration = 99999*time.Hour + 59*time.Minute + 59*time.Second + 999*time.Millisecond
+
 func FormatDuration(duration time.Duration) (string, error) {
 	/*
 		formatDuration formats a time object to GEP-2257 Duration format to a GEP-2257 Duration Format
@@ -66,18 +67,26 @@ func FormatDuration(duration time.Duration) (string, error) {
 
 		Returns: string or error if duration cannot be expressed as a GEP-2257 Duration format.
 	*/
-	m, _ := time.ParseDuration("0s")
-	if duration == m {
+	if duration == 0 {
 		return "0s", nil
 	}
+
+	// check if a negative value
+	if duration < 0 {
+		return "", errors.New("Invalid duration format. Cannot have negative durations")
+	}
+	// check for the maximum value allowed to be expressed
+	if duration > maxDuration {
+		return "", errors.New("Invalid duration format. Duration larger than maximum expression allowed in GEP-2257")
+	}
 	// time.Duration allows for floating point ms, which is not allowed in GEP-2257
 	durationMicroseconds := duration.Microseconds()
 
 	if durationMicroseconds%1000 != 0 {
 		return "", errors.New("Cannot express sub-milliseconds precision in GEP-2257")
 	}
 
-	//Golang's time.Duration allows for floating point seconds instead of converting to ms
+	// Golang's time.Duration allows for floating point seconds instead of converting to ms
 	durationMilliseconds := duration.Milliseconds()
 
 	var ms int64
@@ -92,11 +101,6 @@ func FormatDuration(duration time.Duration) (string, error) {
 		durationString += fmt.Sprintf("%dms", ms)
 	}
 
-	// check if a negative value
-	if duration < 0 {
-		return "", errors.New("Invalid duration format. Cannot have negative durations")
-	}
-
 	// trim the 0 values from the string (for example, 30m0s should result in 30m)
 	// going to have a regexp that finds the index of the time units with 0, then appropriately trim those away
 	temp := reSplit.FindAll([]byte(durationString), -1)
@@ -110,7 +114,7 @@ func FormatDuration(duration time.Duration) (string, error) {
 	}
 
 	// check if there are floating number points
-	if matched := re.MatchString(res); matched == false {
+	if !re.MatchString(res) {
 		return "", errors.New("Invalid duration format")
 	}
 

pkg/utils/duration_test.go

@@ -1,13 +1,29 @@
 package main
 
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
 import (
-	"github.com/stretchr/testify/assert"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
-func makeDuration(h, m, s, ms int) time.Duration {
-	duration := h*int(time.Hour) + m*int(time.Minute) + s*int(time.Second) + ms*int(time.Millisecond)
+func makeDuration(h, m, s, ms float64) time.Duration {
+	duration := h*float64(time.Hour) + m*float64(time.Minute) + s*float64(time.Second) + ms*float64(time.Millisecond)
 	return time.Duration(duration)
 }
 
@@ -21,12 +37,12 @@ func TestParseDuration(t *testing.T) {
 		{
 			name:     "0h to timeDuration of 0s",
 			args:     "0h",
-			expected: makeDuration(0, 0, 0, 0),
+			expected: time.Hour * 0,
 		},
 		{
 			name:     "0s should be 0s",
 			args:     "0s",
-			expected: makeDuration(0, 0, 0, 0),
+			expected: time.Second * 0,
 		},
 		{
 			name:     "0h0m0s should be 0s",
@@ -69,7 +85,7 @@ func TestParseDuration(t *testing.T) {
 			expected: makeDuration(0, 0, 7320, 0),
 		},
 		{
-			name:     "1h30m10s shoulw be 1h30m10s",
+			name:     "1h30m10s should be 1h30m10s",
 			args:     "1h30m10s",
 			expected: makeDuration(1, 30, 10, 0),
 		},
@@ -139,112 +155,114 @@ func TestParseDuration(t *testing.T) {
 func TestFormatDuration(t *testing.T) {
 	validTestCases := []struct {
 		name     string
-		args     string
+		args     time.Duration
 		expected string
 	}{
 		{
 			name:     "0 should be 0s",
-			args:     "0",
+			args:     makeDuration(0, 0, 0, 0),
 			expected: "0s",
 		},
 		{
 			name:     "1h should be 1h",
-			args:     "1h",
+			args:     makeDuration(1, 0, 0, 0),
 			expected: "1h",
 		},
 		{
 			name:     "30m should be 30m",
-			args:     "30m",
+			args:     makeDuration(0, 30, 0, 0),
 			expected: "30m",
 		},
 		{
 			name:     "10s should be 10s",
-			args:     "10s",
+			args:     makeDuration(0, 0, 10, 0),
 			expected: "10s",
 		},
 		{
 			name:     "500ms should be 500ms",
-			args:     "500ms",
+			args:     makeDuration(0, 0, 0, 500),
 			expected: "500ms",
 		},
 		{
 			name:     "2h30m should be 2h30m",
-			args:     "2h30m",
+			args:     makeDuration(2, 30, 0, 0),
 			expected: "2h30m",
 		},
 		{
 			name:     "1h30m10s should be 1h30m10s",
-			args:     "1h30m10s",
+			args:     makeDuration(1, 30, 10, 0),
 			expected: "1h30m10s",
 		},
 		{
 			name:     "600ms should be 600ms",
-			args:     "600ms",
+			args:     makeDuration(0, 0, 0, 600),
 			expected: "600ms",
 		},
 		{
 			name:     "2h600ms should be 2h600ms",
-			args:     "2h600ms",
+			args:     makeDuration(2, 0, 0, 600),
 			expected: "2h600ms",
 		},
 		{
 			name:     "2h30m600ms should be 2h30m600ms",
-			args:     "2h30m600ms",
+			args:     makeDuration(2, 30, 0, 600),
 			expected: "2h30m600ms",
 		},
 		{
 			name:     "2h30m10s600ms should be 2h30m10s600ms",
-			args:     "2h30m10s600ms",
+			args:     makeDuration(2, 30, 10, 600),
 			expected: "2h30m10s600ms",
 		},
 		{
 			name:     "0.5m should be 30s",
-			args:     "0.5m",
+			args:     makeDuration(0, 0.5, 0, 0),
 			expected: "30s",
 		},
 		{
 			name:     "0.5s should be 500ms",
-			args:     "0.5s",
+			args:     makeDuration(0, 0, 0.5, 0),
 			expected: "500ms",
 		},
 	}
-
 	// invalid test cases
 	invalidTestCases := []struct {
 		name string
-		args string
+		args time.Duration
 	}{
 		{
 			name: "Sub-milliseconds not allowed (100us)",
-			args: "100us",
+			args: 100 * time.Microsecond,
 		},
 		{
 			name: "Sub-milliseconds not allowed (0.5ms)",
-			args: "0.5ms",
+			args: makeDuration(0, 0, 0, 0.5),
 		},
 		{
 			name: "Out of range (greater than 99999 hours)",
-			args: "100000h",
+			args: makeDuration(100000, 0, 0, 0),
 		},
 		{
 			name: "Negative duration not supported",
-			args: "-10h",
+			args: -15 * time.Hour,
+		},
+		{
+			name: "Max duration represented in GEP",
+			args: makeDuration(99999, 59, 59, 999) + makeDuration(0, 0, 0, 1),
 		},
 	}
 	// Valid test cases
 
 	for _, tc := range validTestCases {
 		t.Run(tc.name, func(t *testing.T) {
-			a, _ := time.ParseDuration(tc.args)
-			arg, _ := FormatDuration(a)
+			arg, _ := FormatDuration(tc.args)
 			assert.Equal(t, tc.expected, arg)
 		})
 	}
 
 	// Invalid test cases
 	for _, tc := range invalidTestCases {
 		t.Run(tc.name, func(t *testing.T) {
-			_, errArg := ParseDuration(tc.args)
+			_, errArg := FormatDuration(tc.args)
 			assert.Error(t, errArg)
 		})
 	}