Make k8s access explicitly read only (#1101)

* Repleace client.Client with client.Reader

This is more aligned with the actual use abd prevents accidental mutation of k8s objects.

Signed-off-by: Etai Lev Ran <[email protected]>

* Replace client.Client with client.Reader

While this is more aligned with the intent, in some cases the hepler
calls unto gateway API code which is expecting a full fledged client.
In those cases, the original parameter type has been retained (instead
of type casting in the call to Gateway API code)

Signed-off-by: Etai Lev Ran <[email protected]>

---------

Signed-off-by: Etai Lev Ran <[email protected]>

Author: elevran

conformance/utils/kubernetes/helpers.go

@@ -69,7 +69,7 @@ func checkCondition(t *testing.T, conditions []metav1.Condition, expectedConditi
 // InferencePoolMustHaveCondition waits for the specified InferencePool resource
 // to exist and report the expected status condition within one of its parent statuses.
 // It polls the InferencePool's status until the condition is met or the timeout occurs.
-func InferencePoolMustHaveCondition(t *testing.T, c client.Client, poolNN types.NamespacedName, expectedCondition metav1.Condition) {
+func InferencePoolMustHaveCondition(t *testing.T, c client.Reader, poolNN types.NamespacedName, expectedCondition metav1.Condition) {
 	t.Helper() // Marks this function as a test helper
 
 	var timeoutConfig config.InferenceExtensionTimeoutConfig = config.DefaultInferenceExtensionTimeoutConfig()
@@ -159,7 +159,7 @@ func InferencePoolMustHaveCondition(t *testing.T, c client.Client, poolNN types.
 // InferencePoolMustHaveNoParents waits for the specified InferencePool resource
 // to exist and report that it has no parent references in its status.
 // This typically indicates it is no longer referenced by any Gateway API resources.
-func InferencePoolMustHaveNoParents(t *testing.T, c client.Client, poolNN types.NamespacedName) {
+func InferencePoolMustHaveNoParents(t *testing.T, c client.Reader, poolNN types.NamespacedName) {
 	t.Helper()
 
 	var lastObservedPool *inferenceapi.InferencePool
@@ -242,7 +242,7 @@ func HTTPRouteMustBeAcceptedAndResolved(t *testing.T, c client.Client, timeoutCo
 // InferencePoolMustBeAcceptedByParent waits for the specified InferencePool
 // to report an Accepted condition with status True and reason "Accepted"
 // from at least one of its parent Gateways.
-func InferencePoolMustBeAcceptedByParent(t *testing.T, c client.Client, poolNN types.NamespacedName) {
+func InferencePoolMustBeAcceptedByParent(t *testing.T, c client.Reader, poolNN types.NamespacedName) {
 	t.Helper()
 
 	acceptedByParentCondition := metav1.Condition{
@@ -259,7 +259,7 @@ func InferencePoolMustBeAcceptedByParent(t *testing.T, c client.Client, poolNN t
 // InferencePoolMustBeRouteAccepted waits for the specified InferencePool resource
 // to exist and report an Accepted condition with Type=RouteConditionAccepted,
 // Status=True, and Reason=RouteReasonAccepted within one of its parent statuses.
-func InferencePoolMustBeRouteAccepted(t *testing.T, c client.Client, poolNN types.NamespacedName) {
+func InferencePoolMustBeRouteAccepted(t *testing.T, c client.Reader, poolNN types.NamespacedName) {
 	t.Helper()
 
 	expectedPoolCondition := metav1.Condition{
@@ -310,7 +310,7 @@ func GetGatewayEndpoint(t *testing.T, k8sClient client.Client, timeoutConfig gat
 
 // GetPodsWithLabel retrieves a list of Pods.
 // It finds pods matching the given labels in a specific namespace.
-func GetPodsWithLabel(t *testing.T, c client.Client, namespace string, labels map[string]string, timeConfig gatewayapiconfig.TimeoutConfig) ([]corev1.Pod, error) {
+func GetPodsWithLabel(t *testing.T, c client.Reader, namespace string, labels map[string]string, timeConfig gatewayapiconfig.TimeoutConfig) ([]corev1.Pod, error) {
 	t.Helper()
 
 	pods := &corev1.PodList{}

pkg/epp/controller/inferencemodel_reconciler.go

@@ -34,7 +34,7 @@ import (
 )
 
 type InferenceModelReconciler struct {
-	client.Client
+	client.Reader
 	Record             record.EventRecorder
 	Datastore          datastore.Datastore
 	PoolNamespacedName types.NamespacedName
@@ -88,7 +88,7 @@ func (c *InferenceModelReconciler) handleModelDeleted(ctx context.Context, req t
 	logger.Info("InferenceModel removed from datastore", "poolRef", existing.Spec.PoolRef, "modelName", existing.Spec.ModelName)
 
 	// TODO(#409): replace this backfill logic with one that is based on InferenceModel Ready conditions once those are set by an external controller.
-	updated, err := c.Datastore.ModelResync(ctx, c.Client, existing.Spec.ModelName)
+	updated, err := c.Datastore.ModelResync(ctx, c.Reader, existing.Spec.ModelName)
 	if err != nil {
 		return err
 	}

pkg/epp/controller/inferencemodel_reconciler_test.go

@@ -201,7 +201,7 @@ func TestInferenceModelReconciler(t *testing.T) {
 			}
 			_ = ds.PoolSet(context.Background(), fakeClient, pool)
 			reconciler := &InferenceModelReconciler{
-				Client:             fakeClient,
+				Reader:             fakeClient,
 				Record:             record.NewFakeRecorder(10),
 				Datastore:          ds,
 				PoolNamespacedName: types.NamespacedName{Name: pool.Name, Namespace: pool.Namespace},

pkg/epp/controller/inferencepool_reconciler.go

@@ -33,7 +33,7 @@ import (
 // This implementation is just used for reading & maintaining data sync. The Gateway implementation
 // will have the proper controller that will create/manage objects on behalf of the server pool.
 type InferencePoolReconciler struct {
-	client.Client
+	client.Reader
 	Record    record.EventRecorder
 	Datastore datastore.Datastore
 }
@@ -60,7 +60,7 @@ func (c *InferencePoolReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, nil
 	}
 	// update pool in datastore
-	if err := c.Datastore.PoolSet(ctx, c.Client, infPool); err != nil {
+	if err := c.Datastore.PoolSet(ctx, c.Reader, infPool); err != nil {
 		logger.Error(err, "Failed to update datastore")
 		return ctrl.Result{}, err
 	}

pkg/epp/controller/inferencepool_reconciler_test.go

@@ -96,7 +96,7 @@ func TestInferencePoolReconciler(t *testing.T) {
 
 	pmf := backendmetrics.NewPodMetricsFactory(&backendmetrics.FakePodMetricsClient{}, time.Second)
 	datastore := datastore.NewDatastore(ctx, pmf)
-	inferencePoolReconciler := &InferencePoolReconciler{Client: fakeClient, Datastore: datastore}
+	inferencePoolReconciler := &InferencePoolReconciler{Reader: fakeClient, Datastore: datastore}
 
 	// Step 1: Inception, only ready pods matching pool1 are added to the store.
 	if _, err := inferencePoolReconciler.Reconcile(ctx, req); err != nil {

pkg/epp/datastore/datastore.go

@@ -48,7 +48,7 @@ type Datastore interface {
 	// PoolSet sets the given pool in datastore. If the given pool has different label selector than the previous pool
 	// that was stored, the function triggers a resync of the pods to keep the datastore updated. If the given pool
 	// is nil, this call triggers the datastore.Clear() function.
-	PoolSet(ctx context.Context, client client.Client, pool *v1alpha2.InferencePool) error
+	PoolSet(ctx context.Context, reader client.Reader, pool *v1alpha2.InferencePool) error
 	PoolGet() (*v1alpha2.InferencePool, error)
 	PoolHasSynced() bool
 	PoolLabelsMatch(podLabels map[string]string) bool
@@ -57,7 +57,7 @@ type Datastore interface {
 	ModelSetIfOlder(infModel *v1alpha2.InferenceModel) bool
 	ModelGet(modelName string) *v1alpha2.InferenceModel
 	ModelDelete(namespacedName types.NamespacedName) *v1alpha2.InferenceModel
-	ModelResync(ctx context.Context, ctrlClient client.Client, modelName string) (bool, error)
+	ModelResync(ctx context.Context, reader client.Reader, modelName string) (bool, error)
 	ModelGetAll() []*v1alpha2.InferenceModel
 
 	// PodMetrics operations
@@ -110,7 +110,7 @@ func (ds *datastore) Clear() {
 }
 
 // /// InferencePool APIs ///
-func (ds *datastore) PoolSet(ctx context.Context, client client.Client, pool *v1alpha2.InferencePool) error {
+func (ds *datastore) PoolSet(ctx context.Context, reader client.Reader, pool *v1alpha2.InferencePool) error {
 	if pool == nil {
 		ds.Clear()
 		return nil
@@ -129,7 +129,7 @@ func (ds *datastore) PoolSet(ctx context.Context, client client.Client, pool *v1
 		// 2) If the selector on the pool was updated, then we will not get any pod events, and so we need
 		//    to resync the whole pool: remove pods in the store that don't match the new selector and add
 		//    the ones that may have existed already to the store.
-		if err := ds.podResyncAll(ctx, client); err != nil {
+		if err := ds.podResyncAll(ctx, reader); err != nil {
 			return fmt.Errorf("failed to update pods according to the pool selector - %w", err)
 		}
 	}
@@ -182,12 +182,12 @@ func (ds *datastore) ModelSetIfOlder(infModel *v1alpha2.InferenceModel) bool {
 	return true
 }
 
-func (ds *datastore) ModelResync(ctx context.Context, c client.Client, modelName string) (bool, error) {
+func (ds *datastore) ModelResync(ctx context.Context, reader client.Reader, modelName string) (bool, error) {
 	ds.poolAndModelsMu.Lock()
 	defer ds.poolAndModelsMu.Unlock()
 
 	var models v1alpha2.InferenceModelList
-	if err := c.List(ctx, &models, client.MatchingFields{ModelNameIndexKey: modelName}, client.InNamespace(ds.pool.Namespace)); err != nil {
+	if err := reader.List(ctx, &models, client.MatchingFields{ModelNameIndexKey: modelName}, client.InNamespace(ds.pool.Namespace)); err != nil {
 		return false, fmt.Errorf("listing models that match the modelName %s: %w", modelName, err)
 	}
 	if len(models.Items) == 0 {
@@ -288,10 +288,10 @@ func (ds *datastore) PodDelete(namespacedName types.NamespacedName) {
 	}
 }
 
-func (ds *datastore) podResyncAll(ctx context.Context, ctrlClient client.Client) error {
+func (ds *datastore) podResyncAll(ctx context.Context, reader client.Reader) error {
 	logger := log.FromContext(ctx)
 	podList := &corev1.PodList{}
-	if err := ctrlClient.List(ctx, podList, &client.ListOptions{
+	if err := reader.List(ctx, podList, &client.ListOptions{
 		LabelSelector: selectorFromInferencePoolSelector(ds.pool.Spec.Selector),
 		Namespace:     ds.pool.Namespace,
 	}); err != nil {

pkg/epp/server/runserver.go

@@ -93,15 +93,15 @@ func (r *ExtProcServerRunner) SetupWithManager(ctx context.Context, mgr ctrl.Man
 	// Create the controllers and register them with the manager
 	if err := (&controller.InferencePoolReconciler{
 		Datastore: r.Datastore,
-		Client:    mgr.GetClient(),
+		Reader:    mgr.GetClient(),
 		Record:    mgr.GetEventRecorderFor("InferencePool"),
 	}).SetupWithManager(mgr); err != nil {
 		return fmt.Errorf("failed setting up InferencePoolReconciler: %w", err)
 	}
 
 	if err := (&controller.InferenceModelReconciler{
 		Datastore:          r.Datastore,
-		Client:             mgr.GetClient(),
+		Reader:             mgr.GetClient(),
 		PoolNamespacedName: r.PoolNamespacedName,
 		Record:             mgr.GetEventRecorderFor("InferenceModel"),
 	}).SetupWithManager(ctx, mgr); err != nil {