chore: use client.OjectKeyFromObject in client.Get() (#1301)

Signed-off-by: Wen Zhou <[email protected]>

controllers/dscinitialization/monitoring.go

@@ -418,7 +418,7 @@ func createMonitoringProxySecret(ctx context.Context, cli client.Client, name st
 	}
 
 	foundProxySecret := &corev1.Secret{}
-	err = cli.Get(ctx, client.ObjectKey{Name: name, Namespace: dsciInit.Spec.Monitoring.Namespace}, foundProxySecret)
+	err = cli.Get(ctx, client.ObjectKeyFromObject(desiredProxySecret), foundProxySecret)
 	if err != nil {
 		if k8serr.IsNotFound(err) {
 			// Set Controller reference

controllers/dscinitialization/utils.go

@@ -51,7 +51,7 @@ func (r *DSCInitializationReconciler) createOdhNamespace(ctx context.Context, ds
 
 	// Create Application Namespace if it doesn't exist
 	foundNamespace := &corev1.Namespace{}
-	err := r.Get(ctx, client.ObjectKey{Name: name}, foundNamespace)
+	err := r.Get(ctx, client.ObjectKeyFromObject(desiredNamespace), foundNamespace)
 	if err != nil {
 		if k8serr.IsNotFound(err) {
 			log.Info("Creating namespace", "name", name)
@@ -169,10 +169,7 @@ func (r *DSCInitializationReconciler) createDefaultRoleBinding(ctx context.Conte
 
 	// Create RoleBinding if doesn't exists
 	foundRoleBinding := &rbacv1.RoleBinding{}
-	err := r.Client.Get(ctx, client.ObjectKey{
-		Name:      name,
-		Namespace: name,
-	}, foundRoleBinding)
+	err := r.Client.Get(ctx, client.ObjectKeyFromObject(desiredRoleBinding), foundRoleBinding)
 	if err != nil {
 		if k8serr.IsNotFound(err) {
 			// Set Controller reference
@@ -292,10 +289,7 @@ func (r *DSCInitializationReconciler) reconcileDefaultNetworkPolicy(ctx context.
 		// Create NetworkPolicy if it doesn't exist
 		foundNetworkPolicy := &networkingv1.NetworkPolicy{}
 		justCreated := false
-		err := r.Client.Get(ctx, client.ObjectKey{
-			Name:      name,
-			Namespace: name,
-		}, foundNetworkPolicy)
+		err := r.Client.Get(ctx, client.ObjectKeyFromObject(desiredNetworkPolicy), foundNetworkPolicy)
 		if err != nil {
 			if k8serr.IsNotFound(err) {
 				// Set Controller reference
@@ -397,10 +391,7 @@ func (r *DSCInitializationReconciler) createOdhCommonConfigMap(ctx context.Conte
 
 	// Create Configmap if doesn't exists
 	foundConfigMap := &corev1.ConfigMap{}
-	err := r.Client.Get(ctx, client.ObjectKey{
-		Name:      name,
-		Namespace: name,
-	}, foundConfigMap)
+	err := r.Client.Get(ctx, client.ObjectKeyFromObject(desiredConfigMap), foundConfigMap)
 	if err != nil {
 		if k8serr.IsNotFound(err) {
 			// Set Controller reference
@@ -430,10 +421,7 @@ func (r *DSCInitializationReconciler) createUserGroup(ctx context.Context, dscIn
 		// Otherwise is errors with "error": "Group.user.openshift.io \"odh-admins\" is invalid: users: Invalid value: \"null\": users in body must be of type array: \"null\""}
 		Users: []string{},
 	}
-	err := r.Client.Get(ctx, client.ObjectKey{
-		Name:      userGroup.Name,
-		Namespace: dscInit.Spec.ApplicationsNamespace,
-	}, userGroup)
+	err := r.Client.Get(ctx, client.ObjectKeyFromObject(userGroup), userGroup)
 	if err != nil {
 		if k8serr.IsNotFound(err) {
 			err = r.Client.Create(ctx, userGroup)

pkg/cluster/resources.go

@@ -72,7 +72,7 @@ func CreateSecret(ctx context.Context, cli client.Client, name, namespace string
 	}
 
 	foundSecret := &corev1.Secret{}
-	err := cli.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, foundSecret)
+	err := cli.Get(ctx, client.ObjectKeyFromObject(desiredSecret), foundSecret)
 	if err != nil {
 		if k8serr.IsNotFound(err) {
 			err = cli.Create(ctx, desiredSecret)
@@ -100,11 +100,7 @@ func CreateOrUpdateConfigMap(ctx context.Context, c client.Client, desiredCfgMap
 	}
 
 	existingCfgMap := &corev1.ConfigMap{}
-	err := c.Get(ctx, client.ObjectKey{
-		Name:      desiredCfgMap.Name,
-		Namespace: desiredCfgMap.Namespace,
-	}, existingCfgMap)
-
+	err := c.Get(ctx, client.ObjectKeyFromObject(desiredCfgMap), existingCfgMap)
 	if k8serr.IsNotFound(err) {
 		return c.Create(ctx, desiredCfgMap)
 	} else if err != nil {
@@ -144,7 +140,7 @@ func CreateNamespace(ctx context.Context, cli client.Client, namespace string, m
 	}
 
 	foundNamespace := &corev1.Namespace{}
-	if getErr := cli.Get(ctx, client.ObjectKey{Name: namespace}, foundNamespace); client.IgnoreNotFound(getErr) != nil {
+	if getErr := cli.Get(ctx, client.ObjectKeyFromObject(desiredNamespace), foundNamespace); client.IgnoreNotFound(getErr) != nil {
 		return nil, getErr
 	}
 

pkg/cluster/roles.go

@@ -23,7 +23,7 @@ func CreateOrUpdateClusterRole(ctx context.Context, cli client.Client, name stri
 	}
 
 	foundClusterRole := &rbacv1.ClusterRole{}
-	err := cli.Get(ctx, client.ObjectKey{Name: desiredClusterRole.GetName()}, foundClusterRole)
+	err := cli.Get(ctx, client.ObjectKeyFromObject(desiredClusterRole), foundClusterRole)
 	if k8serr.IsNotFound(err) {
 		return desiredClusterRole, cli.Create(ctx, desiredClusterRole)
 	}
@@ -63,7 +63,7 @@ func CreateOrUpdateClusterRoleBinding(ctx context.Context, cli client.Client, na
 	}
 
 	foundClusterRoleBinding := &rbacv1.ClusterRoleBinding{}
-	err := cli.Get(ctx, client.ObjectKey{Name: desiredClusterRoleBinding.GetName()}, foundClusterRoleBinding)
+	err := cli.Get(ctx, client.ObjectKeyFromObject(desiredClusterRoleBinding), foundClusterRoleBinding)
 	if k8serr.IsNotFound(err) {
 		return desiredClusterRoleBinding, cli.Create(ctx, desiredClusterRoleBinding)
 	}

pkg/trustedcabundle/trustedcabundle.go

@@ -71,10 +71,7 @@ func CreateOdhTrustedCABundleConfigMap(ctx context.Context, cli client.Client, n
 
 	// Create Configmap if doesn't exist
 	foundConfigMap := &corev1.ConfigMap{}
-	if err := cli.Get(ctx, client.ObjectKey{
-		Name:      CAConfigMapName,
-		Namespace: namespace,
-	}, foundConfigMap); err != nil {
+	if err := cli.Get(ctx, client.ObjectKeyFromObject(desiredConfigMap), foundConfigMap); err != nil {
 		if k8serr.IsNotFound(err) {
 			err = cli.Create(ctx, desiredConfigMap)
 			if err != nil && !k8serr.IsAlreadyExists(err) {
@@ -105,20 +102,20 @@ func DeleteOdhTrustedCABundleConfigMap(ctx context.Context, cli client.Client, n
 	return cli.Delete(ctx, foundConfigMap)
 }
 
-// IsTrustedCABundleUpdated check if data in CM "odh-trusted-ca-bundle" from applciation namespace matches DSCI's TrustedCABundle.CustomCABundle
+// IsTrustedCABundleUpdated check if data in CM "odh-trusted-ca-bundle" from application namespace matches DSCI's TrustedCABundle.CustomCABundle
 // return false when these two are matching => skip update
 // return true when not match => need upate.
 func IsTrustedCABundleUpdated(ctx context.Context, cli client.Client, dscInit *dsciv1.DSCInitialization) (bool, error) {
-	userNamespace := &corev1.Namespace{}
-	if err := cli.Get(ctx, client.ObjectKey{Name: dscInit.Spec.ApplicationsNamespace}, userNamespace); err != nil {
+	appNamespace := &corev1.Namespace{}
+	if err := cli.Get(ctx, client.ObjectKey{Name: dscInit.Spec.ApplicationsNamespace}, appNamespace); err != nil {
 		if k8serr.IsNotFound(err) {
 			// if namespace is not found, return true. This is to ensure we reconcile, and check for other namespaces.
 			return true, nil
 		}
 		return false, err
 	}
 
-	if !ShouldInjectTrustedBundle(userNamespace) {
+	if !ShouldInjectTrustedBundle(appNamespace) {
 		return false, nil
 	}
 

tests/e2e/deletion_test.go

@@ -7,7 +7,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"k8s.io/apimachinery/pkg/api/errors"
+	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -57,7 +57,7 @@ func (tc *testContext) testDeletionExistDSC() error {
 		if dscerr != nil {
 			return fmt.Errorf("error deleting DSC instance %s: %w", expectedDSC.Name, dscerr)
 		}
-	} else if !errors.IsNotFound(err) {
+	} else if !k8serr.IsNotFound(err) {
 		if err != nil {
 			return fmt.Errorf("could not find DSC instance to delete: %w", err)
 		}
@@ -120,7 +120,7 @@ func (tc *testContext) testDeletionExistDSCI() error {
 		if dscierr != nil {
 			return fmt.Errorf("error deleting DSCI instance %s: %w", expectedDSCI.Name, dscierr)
 		}
-	} else if !errors.IsNotFound(err) {
+	} else if !k8serr.IsNotFound(err) {
 		if err != nil {
 			return fmt.Errorf("could not find DSCI instance to delete :%w", err)
 		}

tests/e2e/helper_test.go

@@ -14,7 +14,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
@@ -59,7 +59,7 @@ func (tc *testContext) waitForOperatorDeployment(name string, replicas int32) er
 	err := wait.PollUntilContextTimeout(tc.ctx, generalRetryInterval, operatorReadyTimeout, false, func(ctx context.Context) (bool, error) {
 		controllerDeployment, err := tc.kubeClient.AppsV1().Deployments(tc.operatorNamespace).Get(ctx, name, metav1.GetOptions{})
 		if err != nil {
-			if errors.IsNotFound(err) {
+			if k8serr.IsNotFound(err) {
 				return false, nil
 			}
 			log.Printf("Failed to get %s controller deployment", name)
@@ -207,7 +207,7 @@ func (tc *testContext) validateCRD(crdName string) error {
 	err := wait.PollUntilContextTimeout(tc.ctx, generalRetryInterval, crdReadyTimeout, false, func(ctx context.Context) (bool, error) {
 		err := tc.customClient.Get(ctx, obj, crd)
 		if err != nil {
-			if errors.IsNotFound(err) {
+			if k8serr.IsNotFound(err) {
 				return false, nil
 			}
 			log.Printf("Failed to get CRD %s", crdName)
@@ -256,7 +256,7 @@ func getCSV(ctx context.Context, cli client.Client, name string, namespace strin
 		}
 	}
 
-	return nil, errors.NewNotFound(schema.GroupResource{}, name)
+	return nil, k8serr.NewNotFound(schema.GroupResource{}, name)
 }
 
 // Use existing or create a new one.
@@ -279,7 +279,7 @@ func getSubscription(tc *testContext, name string, ns string) (*ofapi.Subscripti
 	}
 
 	err := tc.customClient.Get(tc.ctx, key, sub)
-	if errors.IsNotFound(err) {
+	if k8serr.IsNotFound(err) {
 		return createSubscription(name, ns)
 	}
 	if err != nil {
@@ -293,7 +293,7 @@ func waitCSV(tc *testContext, name string, ns string) error {
 	interval := generalRetryInterval
 	isReady := func(ctx context.Context) (bool, error) {
 		csv, err := getCSV(ctx, tc.customClient, name, ns)
-		if errors.IsNotFound(err) {
+		if k8serr.IsNotFound(err) {
 			return false, nil
 		}
 		if err != nil {

tests/integration/features/cleanup_int_test.go

@@ -4,7 +4,7 @@ import (
 	"context"
 
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -81,7 +81,7 @@ var _ = Describe("feature cleanup", func() {
 				WithContext(ctx).
 				WithTimeout(fixtures.Timeout).
 				WithPolling(fixtures.Interval).
-				Should(WithTransform(errors.IsNotFound, BeTrue()))
+				Should(WithTransform(k8serr.IsNotFound, BeTrue()))
 		})
 
 	})
@@ -154,11 +154,11 @@ var _ = Describe("feature cleanup", func() {
 				WithContext(ctx).
 				WithTimeout(fixtures.Timeout).
 				WithPolling(fixtures.Interval).
-				Should(WithTransform(errors.IsNotFound, BeTrue()))
+				Should(WithTransform(k8serr.IsNotFound, BeTrue()))
 
 			Consistently(func() error {
 				_, err := fixtures.GetFeatureTracker(ctx, envTestClient, namespace, featureName)
-				if errors.IsNotFound(err) {
+				if k8serr.IsNotFound(err) {
 					return nil
 				}
 				return err
@@ -213,7 +213,7 @@ func createdSecretHasOwnerReferenceToOwningFeature(namespace, featureName string
 
 func namespaceExists(ctx context.Context, cli client.Client, f *feature.Feature) (bool, error) {
 	namespace, err := fixtures.GetNamespace(ctx, cli, "conditional-ns")
-	if errors.IsNotFound(err) {
+	if k8serr.IsNotFound(err) {
 		return false, nil
 	}
 	// ensuring it fails if namespace is still deleting

tests/integration/features/servicemesh_feature_test.go

@@ -5,7 +5,7 @@ import (
 	"path"
 
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/yaml"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -251,7 +251,7 @@ var _ = Describe("Service Mesh setup", func() {
 							Expect(found).To(BeTrue())
 
 							_, err = fixtures.GetNamespace(ctx, envTestClient, serviceMeshSpec.Auth.Namespace)
-							Expect(errors.IsNotFound(err)).To(BeTrue())
+							Expect(k8serr.IsNotFound(err)).To(BeTrue())
 
 							return extensionProviders