Merge pull request #3489 from kunalmemane/fix-modernize-lint

🌱 Adopt new(x) over ptr.To(x) and re-enable newexpr lint

Author: k8s-ci-robot

.golangci.yml

@@ -81,7 +81,6 @@ linters:
       disable:
         - omitzero
         - fmtappendf
-        - newexpr
     revive:
       rules:
         # The following rules are recommended https://github.com/mgechev/revive#recommended-configuration

designs/warmreplicas.md

@@ -42,7 +42,7 @@ Warmup runnables are also stopped in parallel with non-leader runnables during s
 
 Controllers expose the option via:
 
-- `ctrl.Options{Controller: config.Controller{EnableWarmup: ptr.To(true)}}`
+- `ctrl.Options{Controller: config.Controller{EnableWarmup: new(true)}}`
 
 If both `EnableWarmup` and `NeedLeaderElection` are true, controller-runtime registers the controller as a warmup runnable. Calling `Warmup` launches the same event sources and cache sync logic as `Start`, but it does **not** start worker goroutines. Once the manager becomes leader, the controller’s normal `Start` simply spins up workers against the already-initialized queue. Enabling warmup on a controller that does not use leader election is a no-op, as the worker threads do not block on leader election being won.
 
@@ -51,7 +51,7 @@ If both `EnableWarmup` and `NeedLeaderElection` are true, controller-runtime reg
 ```go
 mgr, err := ctrl.NewManager(cfg, ctrl.Options{
 	Controller: config.Controller{
-		EnableWarmup: ptr.To(true), // make every controller warm up
+		EnableWarmup: new(true), // make every controller warm up
 	},
 })
 if err != nil {
@@ -61,7 +61,7 @@ if err != nil {
 builder.ControllerManagedBy(mgr).
 	Named("slow-source").
 	WithOptions(controller.Options{
-		EnableWarmup: ptr.To(true), // optional per-controller override
+		EnableWarmup: new(true), // optional per-controller override
 	}).
 	For(&examplev1.Example{}).
 	Complete(reconciler)

pkg/builder/controller_test.go

@@ -33,7 +33,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/util/workqueue"
-	"k8s.io/utils/ptr"
 
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -711,7 +710,7 @@ func doReconcileTest(ctx context.Context, nameSuffix string, mgr manager.Manager
 					Name:       deployName,
 					Kind:       "Deployment",
 					APIVersion: "apps/v1",
-					Controller: ptr.To(true),
+					Controller: new(true),
 					UID:        dep.UID,
 				},
 			},

pkg/cache/cache_test.go

@@ -42,7 +42,6 @@ import (
 	kscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	kcache "k8s.io/client-go/tools/cache"
-	"k8s.io/utils/ptr"
 
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -148,10 +147,10 @@ var _ = Describe("Multi-Namespace Informer Cache", func() {
 
 var _ = Describe("Informer Cache without global DeepCopy", func() {
 	CacheTest(cache.New, cache.Options{
-		DefaultUnsafeDisableDeepCopy: ptr.To(true),
+		DefaultUnsafeDisableDeepCopy: new(true),
 	})
 	NonBlockingGetTest(cache.New, cache.Options{
-		DefaultUnsafeDisableDeepCopy: ptr.To(true),
+		DefaultUnsafeDisableDeepCopy: new(true),
 	})
 })
 

pkg/cache/defaulting_test.go

@@ -32,7 +32,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
-	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -205,71 +204,71 @@ func TestDefaultOpts(t *testing.T) {
 			name: "ByObject.UnsafeDisableDeepCopy gets defaulted from DefaultUnsafeDisableDeepCopy",
 			in: Options{
 				ByObject:                     map[client.Object]ByObject{pod: {}},
-				DefaultUnsafeDisableDeepCopy: ptr.To(true),
+				DefaultUnsafeDisableDeepCopy: new(true),
 			},
 
 			verification: func(o Options) string {
-				expected := ptr.To(true)
+				expected := new(true)
 				return cmp.Diff(expected, o.ByObject[pod].UnsafeDisableDeepCopy)
 			},
 		},
 		{
 			name: "ByObject.UnsafeDisableDeepCopy doesn't get defaulted when set",
 			in: Options{
-				ByObject:                     map[client.Object]ByObject{pod: {UnsafeDisableDeepCopy: ptr.To(false)}},
-				DefaultUnsafeDisableDeepCopy: ptr.To(true),
+				ByObject:                     map[client.Object]ByObject{pod: {UnsafeDisableDeepCopy: new(false)}},
+				DefaultUnsafeDisableDeepCopy: new(true),
 			},
 
 			verification: func(o Options) string {
-				expected := ptr.To(false)
+				expected := new(false)
 				return cmp.Diff(expected, o.ByObject[pod].UnsafeDisableDeepCopy)
 			},
 		},
 		{
 			name: "ByObject.EnableWatchBookmarks gets defaulted from DefaultEnableWatchBookmarks",
 			in: Options{
 				ByObject:                    map[client.Object]ByObject{pod: {}},
-				DefaultEnableWatchBookmarks: ptr.To(true),
+				DefaultEnableWatchBookmarks: new(true),
 			},
 
 			verification: func(o Options) string {
-				expected := ptr.To(true)
+				expected := new(true)
 				return cmp.Diff(expected, o.ByObject[pod].EnableWatchBookmarks)
 			},
 		},
 		{
 			name: "ByObject.EnableWatchBookmarks doesn't get defaulted when set",
 			in: Options{
-				ByObject:                    map[client.Object]ByObject{pod: {EnableWatchBookmarks: ptr.To(false)}},
-				DefaultEnableWatchBookmarks: ptr.To(true),
+				ByObject:                    map[client.Object]ByObject{pod: {EnableWatchBookmarks: new(false)}},
+				DefaultEnableWatchBookmarks: new(true),
 			},
 
 			verification: func(o Options) string {
-				expected := ptr.To(false)
+				expected := new(false)
 				return cmp.Diff(expected, o.ByObject[pod].EnableWatchBookmarks)
 			},
 		},
 		{
 			name: "ByObject.SyncPeriod gets defaulted from SyncPeriod",
 			in: Options{
 				ByObject:   map[client.Object]ByObject{pod: {}},
-				SyncPeriod: ptr.To(5 * time.Minute),
+				SyncPeriod: new(5 * time.Minute),
 			},
 
 			verification: func(o Options) string {
-				expected := ptr.To(5 * time.Minute)
+				expected := new(5 * time.Minute)
 				return cmp.Diff(expected, o.ByObject[pod].SyncPeriod)
 			},
 		},
 		{
 			name: "ByObject.SyncPeriod doesn't get defaulted when set",
 			in: Options{
-				ByObject:   map[client.Object]ByObject{pod: {SyncPeriod: ptr.To(1 * time.Minute)}},
-				SyncPeriod: ptr.To(5 * time.Minute),
+				ByObject:   map[client.Object]ByObject{pod: {SyncPeriod: new(1 * time.Minute)}},
+				SyncPeriod: new(5 * time.Minute),
 			},
 
 			verification: func(o Options) string {
-				expected := ptr.To(1 * time.Minute)
+				expected := new(1 * time.Minute)
 				return cmp.Diff(expected, o.ByObject[pod].SyncPeriod)
 			},
 		},

pkg/client/apiutil/restmapper.go

@@ -28,7 +28,6 @@ import (
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/restmapper"
-	"k8s.io/utils/ptr"
 )
 
 // NewDynamicRESTMapper returns a dynamic RESTMapper for cfg. The dynamic
@@ -197,7 +196,7 @@ func (m *mapper) addKnownGroupAndReload(groupName string, versions ...string) er
 				}
 			}
 			if len(failedGroups) > 0 {
-				return ptr.To(ErrResourceDiscoveryFailed(failedGroups))
+				return new(ErrResourceDiscoveryFailed(failedGroups))
 			}
 			return nil
 		}

pkg/client/client_test.go

@@ -516,7 +516,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 
 			Context("with the DryRun option", func() {
 				It("should not create a new object, global option", func(ctx SpecContext) {
-					cl, err := client.New(cfg, client.Options{DryRun: ptr.To(true)})
+					cl, err := client.New(cfg, client.Options{DryRun: new(true)})
 					Expect(err).NotTo(HaveOccurred())
 					Expect(cl).NotTo(BeNil())
 
@@ -1038,7 +1038,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 				Expect(cm.Data).To(BeComparableTo(obj.Data))
 
 				// Apply with ResourceVersion unset
-				obj.ResourceVersion = ptr.To("")
+				obj.ResourceVersion = new("")
 				data = map[string]string{
 					"a-new-key": "a-new-value",
 				}
@@ -1149,7 +1149,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 
 				By("Creating the eviction")
 				eviction := &policyv1.Eviction{
-					DeleteOptions: &metav1.DeleteOptions{GracePeriodSeconds: ptr.To(int64(0))},
+					DeleteOptions: &metav1.DeleteOptions{GracePeriodSeconds: new(int64(0))},
 				}
 				err = cl.SubResource("eviction").Create(ctx, pod, eviction, &client.SubResourceCreateOptions{})
 				Expect((err)).NotTo(HaveOccurred())
@@ -1665,7 +1665,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 				deploymentAC, err := appsv1applyconfigurations.ExtractDeployment(dep, "foo")
 				Expect(err).NotTo(HaveOccurred())
 				deploymentAC.WithStatus(&appsv1applyconfigurations.DeploymentStatusApplyConfiguration{
-					Replicas: ptr.To(int32(1)),
+					Replicas: new(int32(1)),
 				})
 				Expect(cl.Status().Apply(ctx, deploymentAC, client.FieldOwner("foo"))).To(Succeed())
 

pkg/client/dryrun_test.go

@@ -28,7 +28,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	appsv1applyconfigurations "k8s.io/client-go/applyconfigurations/apps/v1"
-	"k8s.io/utils/ptr"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -40,7 +39,7 @@ var _ = Describe("DryRunClient", func() {
 	var ns = "default"
 
 	getClient := func() client.Client {
-		cl, err := client.New(cfg, client.Options{DryRun: ptr.To(true)})
+		cl, err := client.New(cfg, client.Options{DryRun: new(true)})
 		Expect(err).NotTo(HaveOccurred())
 		Expect(cl).NotTo(BeNil())
 		return cl
@@ -266,7 +265,7 @@ var _ = Describe("DryRunClient", func() {
 		deploymentAC, err := appsv1applyconfigurations.ExtractDeployment(dep, "test-owner")
 		Expect(err).NotTo(HaveOccurred())
 		deploymentAC.WithStatus(&appsv1applyconfigurations.DeploymentStatusApplyConfiguration{
-			Replicas: ptr.To(int32(99)),
+			Replicas: new(int32(99)),
 		})
 
 		Expect(getClient().Status().Apply(ctx, deploymentAC, client.FieldOwner("test-owner"))).NotTo(HaveOccurred())
@@ -281,7 +280,7 @@ var _ = Describe("DryRunClient", func() {
 		deploymentAC, err := appsv1applyconfigurations.ExtractDeployment(dep, "test-owner")
 		Expect(err).NotTo(HaveOccurred())
 		deploymentAC.WithStatus(&appsv1applyconfigurations.DeploymentStatusApplyConfiguration{
-			Replicas: ptr.To(int32(99)),
+			Replicas: new(int32(99)),
 		})
 
 		opts := &client.SubResourceApplyOptions{ApplyOptions: client.ApplyOptions{DryRun: []string{"Bye", "Pippa"}}}

pkg/client/fake/client.go

@@ -67,7 +67,6 @@ import (
 	clientgoapplyconfigurations "k8s.io/client-go/applyconfigurations"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/testing"
-	"k8s.io/utils/ptr"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
@@ -1621,13 +1620,13 @@ func extractScale(obj client.Object) (*autoscalingv1.Scale, error) {
 func applyScale(obj client.Object, scale *autoscalingv1.Scale) error {
 	switch obj := obj.(type) {
 	case *appsv1.Deployment:
-		obj.Spec.Replicas = ptr.To(scale.Spec.Replicas)
+		obj.Spec.Replicas = new(scale.Spec.Replicas)
 	case *appsv1.ReplicaSet:
-		obj.Spec.Replicas = ptr.To(scale.Spec.Replicas)
+		obj.Spec.Replicas = new(scale.Spec.Replicas)
 	case *corev1.ReplicationController:
-		obj.Spec.Replicas = ptr.To(scale.Spec.Replicas)
+		obj.Spec.Replicas = new(scale.Spec.Replicas)
 	case *appsv1.StatefulSet:
-		obj.Spec.Replicas = ptr.To(scale.Spec.Replicas)
+		obj.Spec.Replicas = new(scale.Spec.Replicas)
 	default:
 		// TODO: CRDs https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#scale-subresource
 		return fmt.Errorf("unimplemented scale subresource for resource %T", obj)

pkg/client/fake/client_test.go

@@ -50,7 +50,6 @@ import (
 	"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"
 	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
@@ -1727,7 +1726,7 @@ var _ = Describe("Fake client", func() {
 		objOriginal.APIVersion = actual.APIVersion
 		objOriginal.Kind = actual.Kind
 		objOriginal.ResourceVersion = actual.ResourceVersion
-		objOriginal.Spec.Replicas = ptr.To(int32(2))
+		objOriginal.Spec.Replicas = new(int32(2))
 		Expect(cmp.Diff(objOriginal, actual)).To(BeEmpty())
 	})
 
@@ -2590,7 +2589,7 @@ var _ = Describe("Fake client", func() {
 				Name: "foo",
 			},
 			Spec: appsv1.DeploymentSpec{
-				Replicas: ptr.To[int32](2),
+				Replicas: new(int32(2)),
 			},
 		}
 		cl := NewClientBuilder().Build()
@@ -3011,7 +3010,7 @@ var _ = Describe("Fake client", func() {
 		Expect(cm.Data).To(BeComparableTo(map[string]string{"other": "data"}))
 
 		// Apply with ResourceVersion unset
-		obj.ResourceVersion = ptr.To("")
+		obj.ResourceVersion = new("")
 		obj.Data = map[string]string{"another": "data"}
 		Expect(cl.Apply(ctx, obj, &client.ApplyOptions{FieldManager: "test-manager"})).To(Succeed())
 
@@ -3193,7 +3192,7 @@ var _ = Describe("Fake client", func() {
 		Expect(cl.Apply(ctx, obj, client.FieldOwner("foo"))).NotTo(HaveOccurred())
 		// Ideally we should only test for it to not be empty, realistically we will
 		// break ppl if we ever start setting a different value.
-		Expect(obj.ResourceVersion).To(BeEquivalentTo(ptr.To("1")))
+		Expect(obj.ResourceVersion).To(BeEquivalentTo(new("1")))
 	})
 
 	It("ignores a passed resourceVersion on SSA create", func(ctx SpecContext) {
@@ -3204,7 +3203,7 @@ var _ = Describe("Fake client", func() {
 
 		cl := NewClientBuilder().Build()
 		Expect(cl.Apply(ctx, obj, client.FieldOwner("foo"))).NotTo(HaveOccurred())
-		Expect(obj.ResourceVersion).To(BeEquivalentTo(ptr.To("1")))
+		Expect(obj.ResourceVersion).To(BeEquivalentTo(new("1")))
 	})
 
 	It("allows to set deletionTimestamp on an object during SSA create", func(ctx SpecContext) {
@@ -3331,31 +3330,31 @@ var _ = Describe("Fake client", func() {
 				Name: "foo",
 			},
 			Spec: appsv1.DeploymentSpec{
-				Replicas: ptr.To[int32](2),
+				Replicas: new(int32(2)),
 			},
 		},
 		&appsv1.ReplicaSet{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "foo",
 			},
 			Spec: appsv1.ReplicaSetSpec{
-				Replicas: ptr.To[int32](2),
+				Replicas: new(int32(2)),
 			},
 		},
 		&corev1.ReplicationController{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "foo",
 			},
 			Spec: corev1.ReplicationControllerSpec{
-				Replicas: ptr.To[int32](2),
+				Replicas: new(int32(2)),
 			},
 		},
 		&appsv1.StatefulSet{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "foo",
 			},
 			Spec: appsv1.StatefulSetSpec{
-				Replicas: ptr.To[int32](2),
+				Replicas: new(int32(2)),
 			},
 		},
 	}
@@ -3392,16 +3391,16 @@ var _ = Describe("Fake client", func() {
 			switch expected := objExpected.(type) {
 			case *appsv1.Deployment:
 				expected.ResourceVersion = objActual.GetResourceVersion()
-				expected.Spec.Replicas = ptr.To(int32(3))
+				expected.Spec.Replicas = new(int32(3))
 			case *appsv1.ReplicaSet:
 				expected.ResourceVersion = objActual.GetResourceVersion()
-				expected.Spec.Replicas = ptr.To(int32(3))
+				expected.Spec.Replicas = new(int32(3))
 			case *corev1.ReplicationController:
 				expected.ResourceVersion = objActual.GetResourceVersion()
-				expected.Spec.Replicas = ptr.To(int32(3))
+				expected.Spec.Replicas = new(int32(3))
 			case *appsv1.StatefulSet:
 				expected.ResourceVersion = objActual.GetResourceVersion()
-				expected.Spec.Replicas = ptr.To(int32(3))
+				expected.Spec.Replicas = new(int32(3))
 			}
 			Expect(cmp.Diff(objExpected, objActual)).To(BeEmpty())