add storage-initializer uid handling for OpenShift with istio-cni

pkg/webhook/admission/pod/metrics_aggregate_injector.go

@@ -19,6 +19,7 @@ package pod
 import (
 	"encoding/json"
 	"fmt"
+
 	"github.com/kserve/kserve/pkg/constants"
 	v1 "k8s.io/api/core/v1"
 )

pkg/webhook/admission/pod/mutator.go

@@ -62,7 +62,14 @@ func (mutator *Mutator) Handle(ctx context.Context, req admission.Request) admis
 	// For some reason pod namespace is always empty when coming to pod mutator, need to set from admission request
 	pod.Namespace = req.AdmissionRequest.Namespace
 
-	if err := mutator.mutate(pod, configMap); err != nil {
+	targetNs := &v1.Namespace{}
+	err = mutator.Client.Get(context.TODO(), k8types.NamespacedName{Name: pod.Namespace, Namespace: pod.Namespace}, targetNs)
+	if err != nil {
+		log.Error(err, "Failed to get the target namespace", "name", pod.Namespace)
+		return admission.Errored(http.StatusInternalServerError, err)
+	}
+
+	if err := mutator.mutate(pod, configMap, targetNs); err != nil {
 		log.Error(err, "Failed to mutate pod", "name", pod.Labels[constants.InferenceServicePodLabelKey])
 		return admission.Errored(http.StatusInternalServerError, err)
 	}
@@ -76,7 +83,7 @@ func (mutator *Mutator) Handle(ctx context.Context, req admission.Request) admis
 	return admission.PatchResponseFromRaw(req.AdmissionRequest.Object.Raw, patch)
 }
 
-func (mutator *Mutator) mutate(pod *v1.Pod, configMap *v1.ConfigMap) error {
+func (mutator *Mutator) mutate(pod *v1.Pod, configMap *v1.ConfigMap, targetNs *v1.Namespace) error {
 	credentialBuilder := credentials.NewCredentialBulder(mutator.Client, configMap)
 
 	storageInitializerConfig, err := getStorageInitializerConfigs(configMap)
@@ -118,7 +125,9 @@ func (mutator *Mutator) mutate(pod *v1.Pod, configMap *v1.ConfigMap) error {
 
 	mutators := []func(pod *v1.Pod) error{
 		InjectGKEAcceleratorSelector,
-		storageInitializer.InjectStorageInitializer,
+		func(pod *v1.Pod) error {
+			return storageInitializer.InjectStorageInitializer(pod, targetNs)
+		},
 		agentInjector.InjectAgent,
 		metricsAggregator.InjectMetricsAggregator,
 	}

pkg/webhook/admission/pod/storage_initializer_injector.go

@@ -41,6 +41,7 @@ const (
 	PvcURIPrefix                            = "pvc://"
 	PvcSourceMountName                      = "kserve-pvc-source"
 	PvcSourceMountPath                      = "/mnt/pvc"
+	OpenShiftUidRangeAnnotationKey          = "openshift.io/sa.scc.uid-range"
 )
 
 type StorageInitializerConfig struct {
@@ -85,7 +86,7 @@ func getStorageInitializerConfigs(configMap *v1.ConfigMap) (*StorageInitializerC
 // for the serving container in a unified way across storage tech by injecting
 // a provisioning INIT container. This is a work around because KNative does not
 // support INIT containers: https://github.com/knative/serving/issues/4307
-func (mi *StorageInitializerInjector) InjectStorageInitializer(pod *v1.Pod) error {
+func (mi *StorageInitializerInjector) InjectStorageInitializer(pod *v1.Pod, targetNs *v1.Namespace) error {
 	// Only inject if the required annotations are set
 	srcURI, ok := pod.ObjectMeta.Annotations[constants.StorageInitializerSourceUriInternalAnnotationKey]
 	if !ok {
@@ -330,12 +331,37 @@ func (mi *StorageInitializerInjector) InjectStorageInitializer(pod *v1.Pod) erro
 		}
 	}
 
-	// Allow to override the uid for the case where ISTIO CNI with DNS proxy is enabled
-	// See for more: https://istio.io/latest/docs/setup/additional-setup/cni/#compatibility-with-application-init-containers.
+	/*
+		OpenShift uses istio-cni which causes an issue with init-containers when calling external services
+		like S3 or similar. Setting the `uid` for the `storage-initializer` to the same `uid` as the
+		`uid` of the `istio-proxy` resolves the issue.
+
+		With upstream istio the user has the option to set the uid to 1337 described in https://istio.io/latest/docs/setup/additional-setup/cni/#compatibility-with-application-init-containers
+		using the annotation IstioSidecarUIDAnnotationKey.
+
+		In OpenShift the `istio-proxy` always gets assigned the first `uid` from the namespaces
+		`uid` range + 1 (The range is defined in an annotation on the namespace).
+	*/
 	if value, ok := pod.GetAnnotations()[constants.IstioSidecarUIDAnnotationKey]; ok {
 		if uid, err := strconv.ParseInt(value, 10, 64); err == nil {
+			if initContainer.SecurityContext == nil {
+				initContainer.SecurityContext = &v1.SecurityContext{}
+			}
 			initContainer.SecurityContext.RunAsUser = ptr.Int64(uid)
 		}
+	} else {
+		uidStr := targetNs.Annotations[OpenShiftUidRangeAnnotationKey]
+		if uidStr != "" {
+			uidStrParts := strings.Split(uidStr, "/")
+			if uid, err := strconv.ParseInt(uidStrParts[0], 10, 64); err == nil {
+				// Set the uid to the first uid in the namespaces range + 1
+				uid++
+				if initContainer.SecurityContext == nil {
+					initContainer.SecurityContext = &v1.SecurityContext{}
+				}
+				initContainer.SecurityContext.RunAsUser = ptr.Int64(uid)
+			}
+		}
 	}
 
 	// Add init container to the spec