Merge pull request #3282 from alvaroaleman/validate-managed-fields

🐛 Fakeclient: Validate managed fields on init objects

Author: k8s-ci-robot

pkg/client/fake/client.go

@@ -81,8 +81,9 @@ import (
 
 type versionedTracker struct {
 	testing.ObjectTracker
-	scheme                *runtime.Scheme
-	withStatusSubresource sets.Set[schema.GroupVersionKind]
+	scheme                        *runtime.Scheme
+	withStatusSubresource         sets.Set[schema.GroupVersionKind]
+	usesFieldManagedObjectTracker bool
 }
 
 type fakeClient struct {
@@ -286,6 +287,7 @@ func (f *ClientBuilder) Build() client.WithWatch {
 		panic(errors.New("WithObjectTracker and WithTypeConverters are incompatible"))
 	}
 
+	var usesFieldManagedObjectTracker bool
 	if f.objectTracker == nil {
 		if len(f.typeConverters) == 0 {
 			// Use corresponding scheme to ensure the converter error
@@ -304,11 +306,13 @@ func (f *ClientBuilder) Build() client.WithWatch {
 			serializer.NewCodecFactory(f.scheme).UniversalDecoder(),
 			multiTypeConverter{upstream: f.typeConverters},
 		)
+		usesFieldManagedObjectTracker = true
 	}
 	tracker := versionedTracker{
-		ObjectTracker:         f.objectTracker,
-		scheme:                f.scheme,
-		withStatusSubresource: withStatusSubResource,
+		ObjectTracker:                 f.objectTracker,
+		scheme:                        f.scheme,
+		withStatusSubresource:         withStatusSubResource,
+		usesFieldManagedObjectTracker: usesFieldManagedObjectTracker,
 	}
 
 	for _, obj := range f.initObject {
@@ -376,6 +380,16 @@ func (t versionedTracker) Add(obj runtime.Object) error {
 		if err != nil {
 			return err
 		}
+
+		// If the fieldManager can not decode fields, it will just silently clear them. This is pretty
+		// much guaranteed not to be what someone that initializes a fake client with objects that
+		// have them set wants, so validate them here.
+		// Ref https://github.com/kubernetes/kubernetes/blob/a956ef4862993b825bcd524a19260192ff1da72d/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/fieldmanager.go#L105
+		if t.usesFieldManagedObjectTracker {
+			if err := managedfields.ValidateManagedFields(accessor.GetManagedFields()); err != nil {
+				return fmt.Errorf("invalid managedFields on %T: %w", obj, err)
+			}
+		}
 		if err := t.ObjectTracker.Add(obj); err != nil {
 			return err
 		}

pkg/client/fake/client_test.go

@@ -42,12 +42,14 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
 	clientgoapplyconfigurations "k8s.io/client-go/applyconfigurations"
 	corev1applyconfigurations "k8s.io/client-go/applyconfigurations/core/v1"
 	"k8s.io/client-go/kubernetes/fake"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/testing"
 	"k8s.io/utils/ptr"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -2839,6 +2841,93 @@ var _ = Describe("Fake client", func() {
 		Expect(*obj.DeletionTimestamp).To(BeEquivalentTo(now))
 	})
 
+	It("will error out if an object with invalid managedFields is added", func(ctx SpecContext) {
+		fieldV1Map := map[string]interface{}{
+			"f:metadata": map[string]interface{}{
+				"f:name":        map[string]interface{}{},
+				"f:labels":      map[string]interface{}{},
+				"f:annotations": map[string]interface{}{},
+				"f:finalizers":  map[string]interface{}{},
+			},
+		}
+		fieldV1, err := json.Marshal(fieldV1Map)
+		Expect(err).NotTo(HaveOccurred())
+
+		obj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{
+			Name:      "cm-1",
+			Namespace: "default",
+			ManagedFields: []metav1.ManagedFieldsEntry{{
+				Manager:    "my-manager",
+				Operation:  metav1.ManagedFieldsOperationUpdate,
+				FieldsType: "FieldsV1",
+				FieldsV1:   &metav1.FieldsV1{Raw: fieldV1},
+			}},
+		}}
+
+		Expect(func() {
+			NewClientBuilder().WithObjects(obj).Build()
+		}).To(PanicWith(MatchError(ContainSubstring("invalid managedFields"))))
+	})
+
+	It("allows adding an object with managedFields", func(ctx SpecContext) {
+		fieldV1Map := map[string]interface{}{
+			"f:metadata": map[string]interface{}{
+				"f:name":        map[string]interface{}{},
+				"f:labels":      map[string]interface{}{},
+				"f:annotations": map[string]interface{}{},
+				"f:finalizers":  map[string]interface{}{},
+			},
+		}
+		fieldV1, err := json.Marshal(fieldV1Map)
+		Expect(err).NotTo(HaveOccurred())
+
+		obj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{
+			Name:      "cm-1",
+			Namespace: "default",
+			ManagedFields: []metav1.ManagedFieldsEntry{{
+				Manager:    "my-manager",
+				Operation:  metav1.ManagedFieldsOperationUpdate,
+				FieldsType: "FieldsV1",
+				FieldsV1:   &metav1.FieldsV1{Raw: fieldV1},
+				APIVersion: "v1",
+			}},
+		}}
+
+		NewClientBuilder().WithObjects(obj).Build()
+	})
+
+	It("allows adding an object with invalid managedFields when not using the FieldManagedObjectTracker", func(ctx SpecContext) {
+		fieldV1Map := map[string]interface{}{
+			"f:metadata": map[string]interface{}{
+				"f:name":        map[string]interface{}{},
+				"f:labels":      map[string]interface{}{},
+				"f:annotations": map[string]interface{}{},
+				"f:finalizers":  map[string]interface{}{},
+			},
+		}
+		fieldV1, err := json.Marshal(fieldV1Map)
+		Expect(err).NotTo(HaveOccurred())
+
+		obj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{
+			Name:      "cm-1",
+			Namespace: "default",
+			ManagedFields: []metav1.ManagedFieldsEntry{{
+				Manager:    "my-manager",
+				Operation:  metav1.ManagedFieldsOperationUpdate,
+				FieldsType: "FieldsV1",
+				FieldsV1:   &metav1.FieldsV1{Raw: fieldV1},
+			}},
+		}}
+
+		NewClientBuilder().
+			WithObjectTracker(testing.NewObjectTracker(
+				clientgoscheme.Scheme,
+				serializer.NewCodecFactory(clientgoscheme.Scheme).UniversalDecoder(),
+			)).
+			WithObjects(obj).
+			Build()
+	})
+
 	scalableObjs := []client.Object{
 		&appsv1.Deployment{
 			ObjectMeta: metav1.ObjectMeta{