Merge pull request #137473 from jpbetz/fix-set-transform

Fix SetTransform to correctly override per-informer transforms

pkg/scheduler/scheduler_test.go

@@ -29,6 +29,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/onsi/gomega"
+	"github.com/stretchr/testify/require"
 
 	v1 "k8s.io/api/core/v1"
 	eventsv1 "k8s.io/api/events/v1"
@@ -1463,3 +1464,32 @@ func (f fakePermitPlugin) Permit(ctx context.Context, state fwk.CycleState, p *v
 }
 
 var _ fwk.PermitPlugin = &fakePermitPlugin{}
+
+func TestNewInformerFactoryTrim(t *testing.T) {
+	cs := fake.NewClientset()
+
+	pd := &v1.Pod{ObjectMeta: metav1.ObjectMeta{
+		Name:      "test",
+		Namespace: "default",
+		ManagedFields: []metav1.ManagedFieldsEntry{
+			{
+				Manager:    "update",
+				Operation:  metav1.ManagedFieldsOperationApply,
+				APIVersion: "apps/v1",
+			},
+		},
+	}}
+	require.NoError(t, cs.Tracker().Add(pd))
+
+	informerFactory := NewInformerFactory(cs, 0)
+	lister := informerFactory.Core().V1().Pods().Lister()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	informerFactory.Start(ctx.Done())
+	informerFactory.WaitForCacheSync(ctx.Done())
+
+	p, err := lister.Pods("default").Get("test")
+	require.NoError(t, err)
+	require.Empty(t, p.GetManagedFields(), "expected managedFields to be trimmed by the transform")
+}

staging/src/k8s.io/apiextensions-apiserver/examples/client-go/pkg/client/informers/externalversions/factory.go

@@ -237,7 +237,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer

staging/src/k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/factory.go

@@ -237,7 +237,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer

staging/src/k8s.io/client-go/informers/factory.go

@@ -256,7 +256,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer

staging/src/k8s.io/code-generator/cmd/informer-gen/generators/factory.go

@@ -313,7 +313,9 @@ func (f *sharedInformerFactory) InformerFor(obj {{.runtimeObject|raw}}, newFunc
   }
 
   informer = newFunc(f.client, resyncPeriod)
-  informer.SetTransform(f.transform)
+  if f.transform != nil {
+    informer.SetTransform(f.transform)
+  }
   f.informers[informerType] = informer
 
   return informer

staging/src/k8s.io/code-generator/examples/HyphenGroup/informers/externalversions/factory.go

@@ -237,7 +237,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer

staging/src/k8s.io/code-generator/examples/MixedCase/informers/externalversions/factory.go

@@ -237,7 +237,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer

staging/src/k8s.io/code-generator/examples/apiserver/informers/externalversions/factory.go

@@ -240,7 +240,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer

staging/src/k8s.io/code-generator/examples/crd/informers/externalversions/factory.go

@@ -240,7 +240,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer

staging/src/k8s.io/code-generator/examples/single/informers/externalversions/factory.go

@@ -237,7 +237,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer

staging/src/k8s.io/code-generator/examples/single/informers/externalversions/factory_test.go

@@ -0,0 +1,111 @@
+/*
+Copyright The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package externalversions
+
+import (
+	"slices"
+	"testing"
+	"time"
+
+	"k8s.io/client-go/tools/cache"
+	singleapiv1 "k8s.io/code-generator/examples/single/api/v1"
+	"k8s.io/code-generator/examples/single/clientset/versioned"
+)
+
+// TestTransforms verified that transform calls are applied as expected.
+func TestTransforms(t *testing.T) {
+	tests := []struct {
+		name                    string
+		useFactoryTransform     bool
+		usePerInformerTransform bool
+		wantTransformCalls      []string
+	}{
+		{
+			name:               "no transforms",
+			wantTransformCalls: nil,
+		},
+		{
+			name:                    "no factory transform preserves per-informer transform",
+			usePerInformerTransform: true,
+			wantTransformCalls:      []string{"per-informer"},
+		},
+		{
+			name:                "factory transform applied when no per-informer transform",
+			useFactoryTransform: true,
+			wantTransformCalls:  []string{"factory"},
+		},
+		{
+			name:                    "factory transform overrides per-informer transform",
+			useFactoryTransform:     true,
+			usePerInformerTransform: true,
+			wantTransformCalls:      []string{"factory"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var transformCalls []string
+			makeTransform := func(name string) cache.TransformFunc {
+				return func(obj interface{}) (interface{}, error) {
+					transformCalls = append(transformCalls, name)
+					return obj, nil
+				}
+			}
+
+			var opts []SharedInformerOption
+			if tt.useFactoryTransform {
+				opts = append(opts, WithTransform(makeTransform("factory")))
+			}
+			factory := NewSharedInformerFactoryWithOptions(nil, 0, opts...)
+
+			var wrapper *transformTrackingInformer
+			newFunc := func(_ versioned.Interface, resyncPeriod time.Duration) cache.SharedIndexInformer {
+				inner := cache.NewSharedIndexInformer(nil, &singleapiv1.TestType{}, resyncPeriod, cache.Indexers{})
+				wrapper = &transformTrackingInformer{SharedIndexInformer: inner}
+				if tt.usePerInformerTransform {
+					err := wrapper.SetTransform(makeTransform("per-informer"))
+					if err != nil {
+						t.Fatalf("failed to set transform: %v", err)
+					}
+				}
+				return wrapper
+			}
+			factory.InformerFor(&singleapiv1.TestType{}, newFunc)
+
+			if wrapper.lastTransform != nil {
+				_, err := wrapper.lastTransform(nil)
+				if err != nil {
+					t.Fatalf("failed to invoke transform: %v", err)
+				}
+			}
+
+			if !slices.Equal(transformCalls, tt.wantTransformCalls) {
+				t.Errorf("transform calls: got %v, want %v", transformCalls, tt.wantTransformCalls)
+			}
+		})
+	}
+}
+
+type transformTrackingInformer struct {
+	cache.SharedIndexInformer
+	lastTransform cache.TransformFunc
+}
+
+func (s *transformTrackingInformer) SetTransform(handler cache.TransformFunc) error {
+	s.lastTransform = handler
+	return s.SharedIndexInformer.SetTransform(handler)
+}

staging/src/k8s.io/kube-aggregator/pkg/client/informers/externalversions/factory.go

@@ -237,7 +237,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer

staging/src/k8s.io/sample-apiserver/pkg/generated/informers/externalversions/factory.go

@@ -237,7 +237,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer

staging/src/k8s.io/sample-controller/pkg/generated/informers/externalversions/factory.go

@@ -237,7 +237,9 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal
 	}
 
 	informer = newFunc(f.client, resyncPeriod)
-	informer.SetTransform(f.transform)
+	if f.transform != nil {
+		informer.SetTransform(f.transform)
+	}
 	f.informers[informerType] = informer
 
 	return informer