Skip to content

Commit cb52769

Browse files
authored
Refactor: Improve env utility (#803)
Refactored the environment variable utility (pkg/epp/util/env) to enhance code quality, readability, and maintainability. Key changes: - Introduced generic helper functions `parseEnvWithValue` and `getEnvWithParser` to centralize common logic for fetching and parsing environment variables, significantly reducing code duplication. - Standardized logging messages for consistency across all `GetEnv<Type>` functions. - Added `GetEnvDuration`.
1 parent 2ed990b commit cb52769

File tree

2 files changed

+119
-34
lines changed

2 files changed

+119
-34
lines changed

pkg/epp/util/env/env.go

+44-34
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,71 @@
11
package env
22

33
import (
4+
"fmt"
45
"os"
6+
"reflect"
57
"strconv"
8+
"time"
69

710
"github.com/go-logr/logr"
811
)
912

10-
// getEnvFloat gets a float64 from an environment variable with a default value
11-
func GetEnvFloat(key string, defaultVal float64, logger logr.Logger) float64 {
12-
val, exists := os.LookupEnv(key)
13-
if !exists {
14-
logger.Info("Environment variable not set, using default value",
15-
"key", key, "defaultValue", defaultVal)
16-
return defaultVal
17-
}
18-
19-
floatVal, err := strconv.ParseFloat(val, 64)
13+
// parseEnvWithValue attempts to parse a string value using the provided
14+
// parser.
15+
// It logs success or failure and returns the parsed value or the default
16+
// value.
17+
// This helper is used when the environment variable is confirmed to exist.
18+
func parseEnvWithValue[T any](key string, valueStr string, defaultVal T,
19+
parser func(string) (T, error), logger logr.Logger) T {
20+
parsedVal, err := parser(valueStr)
2021
if err != nil {
21-
logger.Info("Failed to parse environment variable as float, using default value",
22-
"key", key, "value", val, "error", err, "defaultValue", defaultVal)
22+
logger.Info(fmt.Sprintf("Failed to parse environment variable as %s, using default value", reflect.TypeOf(defaultVal)),
23+
"key", key, "rawValue", valueStr, "error", err,
24+
"defaultValue", defaultVal)
2325
return defaultVal
2426
}
2527

2628
logger.Info("Successfully loaded environment variable",
27-
"key", key, "value", floatVal)
28-
return floatVal
29+
"key", key, "value", parsedVal)
30+
return parsedVal
2931
}
3032

31-
// getEnvInt gets an int from an environment variable with a default value
32-
func GetEnvInt(key string, defaultVal int, logger logr.Logger) int {
33-
val, exists := os.LookupEnv(key)
33+
// getEnvWithParser retrieves an environment variable. If set, it uses the
34+
// provided parser to convert it. Otherwise, it returns the default value.
35+
// It delegates to parseEnvWithValue for the actual parsing and detailed
36+
// logging once the variable is confirmed to exist.
37+
func getEnvWithParser[T any](key string, defaultVal T,
38+
parser func(string) (T, error), logger logr.Logger) T {
39+
valueStr, exists := os.LookupEnv(key)
3440
if !exists {
3541
logger.Info("Environment variable not set, using default value",
3642
"key", key, "defaultValue", defaultVal)
3743
return defaultVal
3844
}
45+
return parseEnvWithValue(key, valueStr, defaultVal, parser, logger)
46+
}
3947

40-
intVal, err := strconv.Atoi(val)
41-
if err != nil {
42-
logger.Info("Failed to parse environment variable as int, using default value",
43-
"key", key, "value", val, "error", err, "defaultValue", defaultVal)
44-
return defaultVal
45-
}
48+
// GetEnvFloat gets a float64 from an environment variable with a default
49+
// value.
50+
func GetEnvFloat(key string, defaultVal float64, logger logr.Logger) float64 {
51+
parser := func(s string) (float64, error) { return strconv.ParseFloat(s, 64) }
52+
return getEnvWithParser(key, defaultVal, parser, logger)
53+
}
4654

47-
logger.Info("Successfully loaded environment variable",
48-
"key", key, "value", intVal)
49-
return intVal
55+
// GetEnvInt gets an int from an environment variable with a default value.
56+
func GetEnvInt(key string, defaultVal int, logger logr.Logger) int {
57+
return getEnvWithParser(key, defaultVal, strconv.Atoi, logger)
58+
}
59+
60+
// GetEnvDuration gets a time.Duration from an environment variable with a
61+
// default value.
62+
func GetEnvDuration(key string, defaultVal time.Duration, logger logr.Logger) time.Duration {
63+
return getEnvWithParser(key, defaultVal, time.ParseDuration, logger)
5064
}
5165

52-
// GetEnvString gets a string from an environment variable with a default value
66+
// GetEnvString gets a string from an environment variable with a default
67+
// value.
5368
func GetEnvString(key string, defaultVal string, logger logr.Logger) string {
54-
val, exists := os.LookupEnv(key)
55-
if !exists {
56-
logger.Info("Environment variable not set, using default value",
57-
"key", key, "defaultValue", defaultVal)
58-
return defaultVal
59-
}
60-
return val
69+
parser := func(s string) (string, error) { return s, nil }
70+
return getEnvWithParser(key, defaultVal, parser, logger)
6171
}

pkg/epp/util/env/env_test.go

+75
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package env
33
import (
44
"os"
55
"testing"
6+
"time"
67

78
"github.com/go-logr/logr/testr"
89
logutil "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/util/logging"
@@ -69,6 +70,80 @@ func TestGetEnvFloat(t *testing.T) {
6970
}
7071
}
7172

73+
func TestGetEnvDuration(t *testing.T) {
74+
logger := testr.New(t)
75+
76+
tests := []struct {
77+
name string
78+
key string
79+
value string
80+
defaultVal time.Duration
81+
expected time.Duration
82+
setup func()
83+
teardown func()
84+
}{
85+
{
86+
name: "env variable exists and is valid",
87+
key: "TEST_DURATION",
88+
value: "1h30m",
89+
defaultVal: 0,
90+
expected: 1*time.Hour + 30*time.Minute,
91+
setup: func() {
92+
os.Setenv("TEST_DURATION", "1h30m")
93+
},
94+
teardown: func() {
95+
os.Unsetenv("TEST_DURATION")
96+
},
97+
},
98+
{
99+
name: "env variable exists but is invalid",
100+
key: "TEST_DURATION",
101+
value: "invalid-duration",
102+
defaultVal: 5 * time.Minute,
103+
expected: 5 * time.Minute,
104+
setup: func() {
105+
os.Setenv("TEST_DURATION", "invalid-duration")
106+
},
107+
teardown: func() {
108+
os.Unsetenv("TEST_DURATION")
109+
},
110+
},
111+
{
112+
name: "env variable does not exist",
113+
key: "TEST_DURATION_MISSING",
114+
defaultVal: 10 * time.Second,
115+
expected: 10 * time.Second,
116+
setup: func() {},
117+
teardown: func() {},
118+
},
119+
{
120+
name: "env variable is empty string",
121+
key: "TEST_DURATION_EMPTY",
122+
value: "",
123+
defaultVal: 1 * time.Millisecond,
124+
expected: 1 * time.Millisecond,
125+
setup: func() {
126+
os.Setenv("TEST_DURATION_EMPTY", "")
127+
},
128+
teardown: func() {
129+
os.Unsetenv("TEST_DURATION_EMPTY")
130+
},
131+
},
132+
}
133+
134+
for _, tc := range tests {
135+
t.Run(tc.name, func(t *testing.T) {
136+
tc.setup()
137+
defer tc.teardown()
138+
139+
result := GetEnvDuration(tc.key, tc.defaultVal, logger.V(logutil.VERBOSE))
140+
if result != tc.expected {
141+
t.Errorf("GetEnvDuration(%s, %v) = %v, expected %v", tc.key, tc.defaultVal, result, tc.expected)
142+
}
143+
})
144+
}
145+
}
146+
72147
func TestGetEnvInt(t *testing.T) {
73148
logger := testr.New(t)
74149

0 commit comments

Comments
 (0)