Skip to content

Commit 2dbe290

Browse files
vertex451Artem Shcherbatiuk
vertex451
and
Artem Shcherbatiuk
authored
fix: handle []interface in change detection of unstructured.Unstructured (#120)
* fix: handle []interface in change detection of unstructured.Unstructured * cleanup --------- Co-authored-by: Artem Shcherbatiuk <[email protected]>
1 parent 6e59052 commit 2dbe290

File tree

2 files changed

+161
-2
lines changed

2 files changed

+161
-2
lines changed

‎gateway/resolver/subscription.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package resolver
22

33
import (
4+
"fmt"
45
v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
56
"reflect"
67
"strings"
@@ -255,6 +256,26 @@ func determineFieldChanged(oldObj, newObj *unstructured.Unstructured, fields []s
255256
// Helper function to get the value of a field from an unstructured object
256257
func getFieldValue(obj *unstructured.Unstructured, fieldPath string) (interface{}, bool, error) {
257258
fields := strings.Split(fieldPath, ".")
258-
value, found, err := unstructured.NestedFieldNoCopy(obj.Object, fields...)
259-
return value, found, err
259+
var current interface{} = obj.Object
260+
261+
for i, field := range fields {
262+
switch v := current.(type) {
263+
case map[string]interface{}:
264+
value, found, err := unstructured.NestedFieldNoCopy(v, field)
265+
if err != nil {
266+
return nil, false, fmt.Errorf("error accessing field %s: %v", strings.Join(fields[:i+1], "."), err)
267+
}
268+
if !found {
269+
return nil, false, nil
270+
}
271+
current = value
272+
case []interface{}:
273+
// in case of slice, we return it, and that slice will be compared later using deep equal
274+
return current, true, nil
275+
default:
276+
return nil, false, fmt.Errorf("unexpected type at field %s, expected map[string]interface{} or []interface{}, got %T", strings.Join(fields[:i+1], "."), current)
277+
}
278+
}
279+
280+
return current, true, nil
260281
}

‎gateway/resolver/subscription_test.go

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
package resolver
2+
3+
import (
4+
"github.com/stretchr/testify/require"
5+
"testing"
6+
7+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
8+
)
9+
10+
func TestDetermineFieldChanged(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
oldObj *unstructured.Unstructured
14+
newObj *unstructured.Unstructured
15+
fields []string
16+
isFieldChanged bool
17+
expectError bool
18+
}{
19+
{
20+
name: "oldObj_is_nil",
21+
oldObj: nil,
22+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
23+
fields: []string{"status.ready"},
24+
isFieldChanged: true,
25+
expectError: false,
26+
},
27+
{
28+
name: "both_objects_are_empty",
29+
oldObj: &unstructured.Unstructured{Object: map[string]interface{}{}},
30+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{}},
31+
fields: []string{"status.ready"},
32+
isFieldChanged: false,
33+
expectError: false,
34+
},
35+
{
36+
name: "field_missing_in_both",
37+
oldObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
38+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
39+
fields: []string{"status.missing"},
40+
isFieldChanged: false,
41+
expectError: false,
42+
},
43+
{
44+
name: "field_present_in_oldObj_but_missing_in_newObj",
45+
oldObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
46+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{}}},
47+
fields: []string{"status.ready"},
48+
isFieldChanged: true,
49+
expectError: false,
50+
},
51+
{
52+
name: "field_present_in_newObj_but_missing_in_oldObj",
53+
oldObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{}}},
54+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
55+
fields: []string{"status.ready"},
56+
isFieldChanged: true,
57+
expectError: false,
58+
},
59+
{
60+
name: "field_value_changed",
61+
oldObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": false}}},
62+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
63+
fields: []string{"status.ready"},
64+
isFieldChanged: true,
65+
expectError: false,
66+
},
67+
{
68+
name: "field_value_changed",
69+
oldObj: &unstructured.Unstructured{Object: map[string]interface{}{
70+
"status": map[string]interface{}{"ready": true, "healthy": true},
71+
}},
72+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{
73+
"status": map[string]interface{}{"ready": true, "healthy": false},
74+
}},
75+
fields: []string{"status.ready", "status.healthy"},
76+
isFieldChanged: true,
77+
expectError: false,
78+
},
79+
{
80+
name: "field_value_unchanged",
81+
oldObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
82+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
83+
fields: []string{"status.ready"},
84+
isFieldChanged: false,
85+
expectError: false,
86+
},
87+
{
88+
name: "nested_field_changed",
89+
oldObj: &unstructured.Unstructured{
90+
Object: map[string]interface{}{
91+
"conditions": []interface{}{map[string]interface{}{"type": "Ready", "status": "True"}},
92+
},
93+
},
94+
newObj: &unstructured.Unstructured{
95+
Object: map[string]interface{}{
96+
"conditions": []interface{}{map[string]interface{}{"type": "Ready", "status": "False"}},
97+
},
98+
},
99+
fields: []string{"conditions.0.status", "conditions.0.type"},
100+
isFieldChanged: true,
101+
expectError: false,
102+
},
103+
{
104+
name: "nested_field_unchanged",
105+
oldObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"conditions": []interface{}{map[string]interface{}{"type": "Ready", "status": "True"}}}}},
106+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"conditions": []interface{}{map[string]interface{}{"type": "Ready", "status": "True"}}}}},
107+
fields: []string{"status.conditions.0.status"},
108+
isFieldChanged: false,
109+
expectError: false,
110+
},
111+
{
112+
name: "invalid_field_path",
113+
oldObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
114+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
115+
fields: []string{"invalid.path"},
116+
isFieldChanged: false,
117+
expectError: false,
118+
},
119+
{
120+
name: "unexpected_type_in_field_path",
121+
oldObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
122+
newObj: &unstructured.Unstructured{Object: map[string]interface{}{"status": map[string]interface{}{"ready": true}}},
123+
fields: []string{"status.ready.invalid"},
124+
isFieldChanged: false,
125+
expectError: true,
126+
},
127+
}
128+
129+
for _, tt := range tests {
130+
t.Run(tt.name, func(t *testing.T) {
131+
result, err := determineFieldChanged(tt.oldObj, tt.newObj, tt.fields)
132+
if tt.expectError {
133+
require.NotNil(t, err)
134+
}
135+
require.Equal(t, tt.isFieldChanged, result)
136+
})
137+
}
138+
}

0 commit comments

Comments
 (0)