Skip to content

Commit 37ebab4

Browse files
committed
fix(validator): robust value comparison and add integration test
valuesEqual now normalizes numeric and boolean types before comparing, preventing false drift reports from YAML int vs JSON float64 (e.g., 1 vs 1.0) and boolean casing (True vs true). Add helm_values_check_test.go integration test wrapper so the registered TestName maps to an actual test function that the validator Job runner can invoke.
1 parent 7f4da79 commit 37ebab4

File tree

3 files changed

+146
-3
lines changed

3 files changed

+146
-3
lines changed

pkg/validator/checks/deployment/helm_values_check.go

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"fmt"
2020
"log/slog"
2121
"sort"
22+
"strconv"
2223
"strings"
2324

2425
"github.com/NVIDIA/aicr/pkg/errors"
@@ -163,7 +164,32 @@ func flattenValuesRecursive(data map[string]any, prefix string, result map[strin
163164
}
164165

165166
// valuesEqual compares two string representations of values, normalizing
166-
// common type differences (e.g., "true"/"true", "1"/"1").
167+
// common type differences between YAML parsing and Helm's JSON storage.
168+
// For example, YAML parses 1 as int while JSON parses it as float64;
169+
// both flatten to "1" but edge cases like "1.0" vs "1" need handling.
167170
func valuesEqual(expected, actual string) bool {
168-
return strings.TrimSpace(expected) == strings.TrimSpace(actual)
171+
expected = strings.TrimSpace(expected)
172+
actual = strings.TrimSpace(actual)
173+
174+
if expected == actual {
175+
return true
176+
}
177+
178+
// Numeric normalization: parse both as float64 to handle
179+
// int vs float differences (YAML int 1 vs JSON float64 1.0).
180+
if ef, ee := strconv.ParseFloat(expected, 64); ee == nil {
181+
if af, ae := strconv.ParseFloat(actual, 64); ae == nil {
182+
return ef == af
183+
}
184+
}
185+
186+
// Boolean normalization: case-insensitive comparison
187+
// handles "True"/"true", "FALSE"/"false".
188+
if eb, ee := strconv.ParseBool(expected); ee == nil {
189+
if ab, ae := strconv.ParseBool(actual); ae == nil {
190+
return eb == ab
191+
}
192+
}
193+
194+
return false
169195
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright (c) 2025, NVIDIA CORPORATION. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package deployment
16+
17+
import (
18+
"testing"
19+
20+
"github.com/NVIDIA/aicr/pkg/validator/checks"
21+
)
22+
23+
// TestCheckHelmValues is the integration test for helm-values.
24+
// This runs inside validator Jobs and invokes the validator.
25+
func TestCheckHelmValues(t *testing.T) {
26+
if testing.Short() {
27+
t.Skip("Skipping integration test in short mode")
28+
}
29+
30+
// Load Job environment
31+
runner, err := checks.NewTestRunner(t)
32+
if err != nil {
33+
t.Skipf("Not in Job environment: %v", err)
34+
}
35+
defer runner.Cancel()
36+
37+
// Check if this check is enabled in recipe
38+
if !runner.HasCheck("deployment", "helm-values") {
39+
t.Skip("Check helm-values not enabled in recipe")
40+
}
41+
42+
t.Logf("Running check: helm-values")
43+
44+
// Run the validator
45+
ctx := runner.Context()
46+
err = validateHelmValues(ctx)
47+
48+
if err != nil {
49+
t.Errorf("Check failed: %v", err)
50+
} else {
51+
t.Logf("✓ Check passed: helm-values")
52+
}
53+
}

pkg/validator/checks/deployment/helm_values_check_unit_test.go

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
"github.com/NVIDIA/aicr/pkg/validator/checks"
2727
)
2828

29-
func TestCheckHelmValues(t *testing.T) {
29+
func TestValidateHelmValues(t *testing.T) {
3030
tests := []struct {
3131
name string
3232
setup func() *checks.ValidationContext
@@ -246,6 +246,38 @@ func TestCheckHelmValues(t *testing.T) {
246246
},
247247
wantErr: false,
248248
},
249+
{
250+
name: "float vs int normalization - 1.0 equals 1",
251+
setup: func() *checks.ValidationContext {
252+
return &checks.ValidationContext{
253+
Context: context.Background(),
254+
Snapshot: snapshotWithHelm(map[string]string{
255+
"gpu-operator.chart": "gpu-operator",
256+
"gpu-operator.values.replicas": "1.0",
257+
}),
258+
Recipe: recipeWithOverrides(map[string]any{
259+
"replicas": 1,
260+
}),
261+
}
262+
},
263+
wantErr: false,
264+
},
265+
{
266+
name: "boolean case normalization - True equals true",
267+
setup: func() *checks.ValidationContext {
268+
return &checks.ValidationContext{
269+
Context: context.Background(),
270+
Snapshot: snapshotWithHelm(map[string]string{
271+
"gpu-operator.chart": "gpu-operator",
272+
"gpu-operator.values.driver.enabled": "True",
273+
}),
274+
Recipe: recipeWithOverrides(map[string]any{
275+
"driver": map[string]any{"enabled": true},
276+
}),
277+
}
278+
},
279+
wantErr: false,
280+
},
249281
{
250282
name: "snapshot key not present for recipe key - skip that key",
251283
setup: func() *checks.ValidationContext {
@@ -402,6 +434,38 @@ func TestFlattenValues(t *testing.T) {
402434
}
403435
}
404436

437+
func TestValuesEqual(t *testing.T) {
438+
tests := []struct {
439+
name string
440+
expected string
441+
actual string
442+
want bool
443+
}{
444+
{"exact match", "foo", "foo", true},
445+
{"whitespace trim", " foo ", "foo", true},
446+
{"different strings", "foo", "bar", false},
447+
{"int vs float same value", "1", "1.0", true},
448+
{"float vs int same value", "3.0", "3", true},
449+
{"different numbers", "1", "2", false},
450+
{"float precision", "3.14", "3.14", true},
451+
{"float mismatch", "3.14", "3.15", false},
452+
{"bool true case insensitive", "true", "True", true},
453+
{"bool false case insensitive", "false", "FALSE", true},
454+
{"bool mismatch", "true", "false", false},
455+
{"bool vs string", "true", "yes", false},
456+
{"numeric string vs non-numeric", "1", "one", false},
457+
{"empty strings", "", "", true},
458+
}
459+
460+
for _, tt := range tests {
461+
t.Run(tt.name, func(t *testing.T) {
462+
if got := valuesEqual(tt.expected, tt.actual); got != tt.want {
463+
t.Errorf("valuesEqual(%q, %q) = %v, want %v", tt.expected, tt.actual, got, tt.want)
464+
}
465+
})
466+
}
467+
}
468+
405469
// snapshotWithHelm creates a snapshot with K8s helm subtype data from flat key-value pairs.
406470
func snapshotWithHelm(data map[string]string) *snapshotter.Snapshot {
407471
helmData := make(map[string]measurement.Reading, len(data))

0 commit comments

Comments
 (0)