Skip to content

Commit 1c33df7

Browse files
committed
🐛 Fakeclient: Validate managed fields on init objects
When the FieldManager in the FieldManagedObjectTracker encounters managed fields it can not decode, it will just silently clear them. This breaks anyone who has tests where the fakeclient gets initialized with objects that have invalid managed fields. This worked prior to using the `FieldManagedObjectTracker`, as the default `testing.ObjectTracker` we used before has no understanding of them. Validate the `managedFields` during fake client initialization to make this issue properly visible.
1 parent c7df6d0 commit 1c33df7

File tree

2 files changed

+108
-5
lines changed

2 files changed

+108
-5
lines changed

pkg/client/fake/client.go

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,9 @@ import (
8181

8282
type versionedTracker struct {
8383
testing.ObjectTracker
84-
scheme *runtime.Scheme
85-
withStatusSubresource sets.Set[schema.GroupVersionKind]
84+
scheme *runtime.Scheme
85+
withStatusSubresource sets.Set[schema.GroupVersionKind]
86+
usesFieldManagedObjectTracker bool
8687
}
8788

8889
type fakeClient struct {
@@ -286,6 +287,7 @@ func (f *ClientBuilder) Build() client.WithWatch {
286287
panic(errors.New("WithObjectTracker and WithTypeConverters are incompatible"))
287288
}
288289

290+
var usesFieldManagedObjectTracker bool
289291
if f.objectTracker == nil {
290292
if len(f.typeConverters) == 0 {
291293
// Use corresponding scheme to ensure the converter error
@@ -304,11 +306,13 @@ func (f *ClientBuilder) Build() client.WithWatch {
304306
serializer.NewCodecFactory(f.scheme).UniversalDecoder(),
305307
multiTypeConverter{upstream: f.typeConverters},
306308
)
309+
usesFieldManagedObjectTracker = true
307310
}
308311
tracker := versionedTracker{
309-
ObjectTracker: f.objectTracker,
310-
scheme: f.scheme,
311-
withStatusSubresource: withStatusSubResource,
312+
ObjectTracker: f.objectTracker,
313+
scheme: f.scheme,
314+
withStatusSubresource: withStatusSubResource,
315+
usesFieldManagedObjectTracker: usesFieldManagedObjectTracker,
312316
}
313317

314318
for _, obj := range f.initObject {
@@ -376,6 +380,16 @@ func (t versionedTracker) Add(obj runtime.Object) error {
376380
if err != nil {
377381
return err
378382
}
383+
384+
// If the fieldManager can not decode fields, it will just silently clear them. This is pretty
385+
// much guaranteed not to be what someone that initializes a fake client with objects that
386+
// have them set wants, so validate them here.
387+
// Ref https://github.com/kubernetes/kubernetes/blob/a956ef4862993b825bcd524a19260192ff1da72d/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/fieldmanager.go#L105
388+
if t.usesFieldManagedObjectTracker {
389+
if err := managedfields.ValidateManagedFields(accessor.GetManagedFields()); err != nil {
390+
return fmt.Errorf("invalid managedFields on %T: %w", obj, err)
391+
}
392+
}
379393
if err := t.ObjectTracker.Add(obj); err != nil {
380394
return err
381395
}

pkg/client/fake/client_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,14 @@ import (
4242
"k8s.io/apimachinery/pkg/labels"
4343
"k8s.io/apimachinery/pkg/runtime"
4444
"k8s.io/apimachinery/pkg/runtime/schema"
45+
"k8s.io/apimachinery/pkg/runtime/serializer"
4546
"k8s.io/apimachinery/pkg/types"
4647
"k8s.io/apimachinery/pkg/watch"
4748
clientgoapplyconfigurations "k8s.io/client-go/applyconfigurations"
4849
corev1applyconfigurations "k8s.io/client-go/applyconfigurations/core/v1"
4950
"k8s.io/client-go/kubernetes/fake"
5051
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
52+
"k8s.io/client-go/testing"
5153
"k8s.io/utils/ptr"
5254

5355
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -2839,6 +2841,93 @@ var _ = Describe("Fake client", func() {
28392841
Expect(*obj.DeletionTimestamp).To(BeEquivalentTo(now))
28402842
})
28412843

2844+
It("will error out if an object with invalid managedFields is added", func(ctx SpecContext) {
2845+
fieldV1Map := map[string]interface{}{
2846+
"f:metadata": map[string]interface{}{
2847+
"f:name": map[string]interface{}{},
2848+
"f:labels": map[string]interface{}{},
2849+
"f:annotations": map[string]interface{}{},
2850+
"f:finalizers": map[string]interface{}{},
2851+
},
2852+
}
2853+
fieldV1, err := json.Marshal(fieldV1Map)
2854+
Expect(err).NotTo(HaveOccurred())
2855+
2856+
obj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{
2857+
Name: "cm-1",
2858+
Namespace: "default",
2859+
ManagedFields: []metav1.ManagedFieldsEntry{{
2860+
Manager: "my-manager",
2861+
Operation: metav1.ManagedFieldsOperationUpdate,
2862+
FieldsType: "FieldsV1",
2863+
FieldsV1: &metav1.FieldsV1{Raw: fieldV1},
2864+
}},
2865+
}}
2866+
2867+
Expect(func() {
2868+
NewClientBuilder().WithObjects(obj).Build()
2869+
}).To(PanicWith(MatchError(ContainSubstring("invalid managedFields"))))
2870+
})
2871+
2872+
It("allows adding an object with managedFields", func(ctx SpecContext) {
2873+
fieldV1Map := map[string]interface{}{
2874+
"f:metadata": map[string]interface{}{
2875+
"f:name": map[string]interface{}{},
2876+
"f:labels": map[string]interface{}{},
2877+
"f:annotations": map[string]interface{}{},
2878+
"f:finalizers": map[string]interface{}{},
2879+
},
2880+
}
2881+
fieldV1, err := json.Marshal(fieldV1Map)
2882+
Expect(err).NotTo(HaveOccurred())
2883+
2884+
obj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{
2885+
Name: "cm-1",
2886+
Namespace: "default",
2887+
ManagedFields: []metav1.ManagedFieldsEntry{{
2888+
Manager: "my-manager",
2889+
Operation: metav1.ManagedFieldsOperationUpdate,
2890+
FieldsType: "FieldsV1",
2891+
FieldsV1: &metav1.FieldsV1{Raw: fieldV1},
2892+
APIVersion: "v1",
2893+
}},
2894+
}}
2895+
2896+
NewClientBuilder().WithObjects(obj).Build()
2897+
})
2898+
2899+
It("allows adding an object with invalid managedFields when not using the FieldManagedObjectTracker", func(ctx SpecContext) {
2900+
fieldV1Map := map[string]interface{}{
2901+
"f:metadata": map[string]interface{}{
2902+
"f:name": map[string]interface{}{},
2903+
"f:labels": map[string]interface{}{},
2904+
"f:annotations": map[string]interface{}{},
2905+
"f:finalizers": map[string]interface{}{},
2906+
},
2907+
}
2908+
fieldV1, err := json.Marshal(fieldV1Map)
2909+
Expect(err).NotTo(HaveOccurred())
2910+
2911+
obj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{
2912+
Name: "cm-1",
2913+
Namespace: "default",
2914+
ManagedFields: []metav1.ManagedFieldsEntry{{
2915+
Manager: "my-manager",
2916+
Operation: metav1.ManagedFieldsOperationUpdate,
2917+
FieldsType: "FieldsV1",
2918+
FieldsV1: &metav1.FieldsV1{Raw: fieldV1},
2919+
}},
2920+
}}
2921+
2922+
NewClientBuilder().
2923+
WithObjectTracker(testing.NewObjectTracker(
2924+
clientgoscheme.Scheme,
2925+
serializer.NewCodecFactory(clientgoscheme.Scheme).UniversalDecoder(),
2926+
)).
2927+
WithObjects(obj).
2928+
Build()
2929+
})
2930+
28422931
scalableObjs := []client.Object{
28432932
&appsv1.Deployment{
28442933
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)