Skip to content

Commit 00a1845

Browse files
committed
Address review comments
Signed-off-by: chiragkyal <ckyal@redhat.com>
1 parent 7dd1849 commit 00a1845

2 files changed

Lines changed: 20 additions & 20 deletions

File tree

pkg/controller/certmanager/deployment_overrides_validation.go

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,9 @@ func withContainerArgsValidateHook(certmanagerinformer certmanagerinformer.CertM
137137
// validatePerformanceArgs performs sanity checks on the performance tuning
138138
// arguments to catch invalid configurations.
139139
func validatePerformanceArgs(argMap map[string]string) error {
140-
// Validate that numeric args are positive integers where applicable.
141-
positiveIntArgs := []string{argConcurrentWorkers, argMaxConcurrentChallenges}
140+
// Validate that integer args are positive.
141+
positiveIntArgs := []string{argConcurrentWorkers, argMaxConcurrentChallenges, argKubeAPIBurst}
142+
parsedInts := make(map[string]int)
142143
for _, arg := range positiveIntArgs {
143144
if valStr, ok := argMap[arg]; ok {
144145
val, err := strconv.Atoi(valStr)
@@ -148,30 +149,29 @@ func validatePerformanceArgs(argMap map[string]string) error {
148149
if val <= 0 {
149150
return fmt.Errorf("validation failed: %s must be greater than 0, got %d", arg, val)
150151
}
152+
parsedInts[arg] = val
151153
}
152154
}
153155

154-
// Validate QPS and burst are numeric and positive.
155-
positiveFloatArgs := []string{argKubeAPIQPS, argKubeAPIBurst}
156-
parsedFloats := make(map[string]float64)
157-
for _, arg := range positiveFloatArgs {
158-
if valStr, ok := argMap[arg]; ok {
159-
val, err := strconv.ParseFloat(valStr, 64)
160-
if err != nil {
161-
return fmt.Errorf("validation failed: %s value must be numeric, got %q", arg, valStr)
162-
}
163-
if val <= 0 {
164-
return fmt.Errorf("validation failed: %s must be greater than 0, got %v", arg, val)
165-
}
166-
parsedFloats[arg] = val
156+
// Validate QPS is a positive float32
157+
var qps float32
158+
var qpsOk bool
159+
if qpsStr, ok := argMap[argKubeAPIQPS]; ok {
160+
val, err := strconv.ParseFloat(qpsStr, 32)
161+
if err != nil {
162+
return fmt.Errorf("validation failed: %s value must be numeric, got %q", argKubeAPIQPS, qpsStr)
163+
}
164+
if val <= 0 {
165+
return fmt.Errorf("validation failed: %s must be greater than 0, got %v", argKubeAPIQPS, val)
167166
}
167+
qps = float32(val)
168+
qpsOk = true
168169
}
169170

170171
// Validate burst >= qps when both are specified.
171-
qps, qpsOk := parsedFloats[argKubeAPIQPS]
172-
burst, burstOk := parsedFloats[argKubeAPIBurst]
173-
if qpsOk && burstOk && burst < qps {
174-
return fmt.Errorf("validation failed: --kube-api-burst (%v) must be >= --kube-api-qps (%v)", burst, qps)
172+
burst, burstOk := parsedInts[argKubeAPIBurst]
173+
if qpsOk && burstOk && float32(burst) < qps {
174+
return fmt.Errorf("validation failed: --kube-api-burst (%d) must be >= --kube-api-qps (%v)", burst, qps)
175175
}
176176

177177
return nil

pkg/controller/certmanager/deployment_overrides_validation_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ func TestWithContainerArgsValidateHook(t *testing.T) {
428428
},
429429
},
430430
deploymentName: certmanagerControllerDeployment,
431-
wantErrMsg: `validation failed: --kube-api-burst value must be numeric, got "xyz"`,
431+
wantErrMsg: `validation failed: --kube-api-burst value must be a positive integer, got "xyz"`,
432432
},
433433
{
434434
name: "controller rejects zero concurrent-workers",

0 commit comments

Comments
 (0)