follow up comments

Signed-off-by: jooho lee <[email protected]>

config/webhook/kustomization.yaml

@@ -13,10 +13,21 @@ patches:
       kind: ValidatingWebhookConfiguration
       name: validating-webhook-configuration
       version: v1
+ - path: mutator_webhook_patch.yaml
+    target:
+      group: admissionregistration.k8s.io
+      kind: MutatingWebhookConfiguration
+      name: mutating-webhook-configuration
+      version: v1      
   - path: field_patch.yaml
     target:
       group: admissionregistration.k8s.io
       kind: ValidatingWebhookConfiguration
       name: validating-webhook-configuration
       version: v1
-
+  - path: mutator_field_patch.yaml
+    target:
+      group: admissionregistration.k8s.io
+      kind: MutatingWebhookConfiguration
+      name: mutating-webhook-configuration
+      version: v1

config/webhook/mutator_field_patch.yaml

@@ -0,0 +1,3 @@
+- op: replace
+  path: /metadata/name
+  value: mutating.odh-model-controller.opendatahub.io

config/webhook/mutator_webhook_patch.yaml

@@ -0,0 +1,11 @@
+apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
+metadata:
+  name: mutating.odh-model-controller.opendatahub.io
+  annotations:
+    service.beta.openshift.io/inject-cabundle: true
+webhooks:
+  - name: mutating.kserve-service.odh-model-controller.opendatahub.io
+    clientConfig:
+      service:
+        name: odh-model-controller-webhook-service

config/webhook/webhook_patch.yaml

@@ -13,4 +13,3 @@ webhooks:
       matchExpressions:
         - key: serving.kserve.io/inferenceservice
           operator: Exists
-

controllers/inferenceservice_controller.go

@@ -32,8 +32,11 @@ import (
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
@@ -106,6 +109,32 @@ func (r *OpenshiftInferenceServiceReconciler) Reconcile(ctx context.Context, req
 	return ctrl.Result{}, err
 }
 
+var certSecretPredicate = predicate.Funcs{
+	UpdateFunc: func(e event.UpdateEvent) bool {
+		return hasInferenceServiceOwner(e.ObjectNew)
+	},
+	CreateFunc: func(e event.CreateEvent) bool {
+		return hasInferenceServiceOwner(e.Object)
+	},
+	DeleteFunc: func(e event.DeleteEvent) bool {
+		return hasInferenceServiceOwner(e.Object)
+	},
+	GenericFunc: func(e event.GenericEvent) bool {
+		return hasInferenceServiceOwner(e.Object)
+	},
+}
+
+// check if the src secret has ownerReferences for InferenceService
+func hasInferenceServiceOwner(obj client.Object) bool {
+	ownerReferences := obj.GetOwnerReferences()
+	for _, ownerReference := range ownerReferences {
+		if ownerReference.Kind == "InferenceService" {
+			return true
+		}
+	}
+	return false
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	builder := ctrl.NewControllerManagedBy(mgr).
@@ -166,7 +195,7 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager)
 				return []reconcile.Request{
 					{NamespacedName: types.NamespacedName{Name: o.GetName(), Namespace: o.GetNamespace()}},
 				}
-			}))
+			}), builder.WithPredicates(certSecretPredicate))
 
 	kserveWithMeshEnabled, kserveWithMeshEnabledErr := utils.VerifyIfComponentIsEnabled(context.Background(), mgr.GetClient(), utils.KServeWithServiceMeshComponent)
 	if kserveWithMeshEnabledErr != nil {

controllers/reconcilers/kserve_isvc_gateway_reconciler.go

@@ -39,8 +39,6 @@ import (
 
 var _ SubResourceReconciler = (*KserveGatewayReconciler)(nil)
 var meshNamespace string
-var destSecretName string
-var portName string
 
 type KserveGatewayReconciler struct {
 	client         client.Client
@@ -62,8 +60,6 @@ func (r *KserveGatewayReconciler) Reconcile(ctx context.Context, log logr.Logger
 	log.V(1).Info("Reconciling KServe local gateway for Kserve InferenceService")
 
 	_, meshNamespace = utils.GetIstioControlPlaneName(ctx, r.client)
-	destSecretName = fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace)
-	portName = fmt.Sprintf("%s-%s", "https", isvc.Name)
 
 	// return if Address.URL is not set
 	if isvc.Status.Address != nil && isvc.Status.Address.URL == nil {
@@ -82,7 +78,7 @@ func (r *KserveGatewayReconciler) Reconcile(ctx context.Context, log logr.Logger
 	}
 
 	// Copy src secret to destination namespace when there is not the synced secret.
-	copiedCertSecret, err := r.secretHandler.Get(ctx, types.NamespacedName{Name: destSecretName, Namespace: meshNamespace})
+	copiedCertSecret, err := r.secretHandler.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace), Namespace: meshNamespace})
 	if err != nil {
 		if errors.IsNotFound(err) {
 			if err := r.copyServingCertSecretFromIsvcNamespace(ctx, srcCertSecret, nil); err != nil {
@@ -137,12 +133,12 @@ func (r *KserveGatewayReconciler) getDesiredResource(isvc *kservev1beta1.Inferen
 				{
 					Hosts: []string{hostname},
 					Port: &istiov1beta1.Port{
-						Name:     portName,
+						Name:     fmt.Sprintf("%s-%s", "https", isvc.Name),
 						Number:   8445,
 						Protocol: "HTTPS",
 					},
 					Tls: &istiov1beta1.ServerTLSSettings{
-						CredentialName: destSecretName,
+						CredentialName: fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace),
 						Mode:           istiov1beta1.ServerTLSSettings_SIMPLE,
 					},
 				},
@@ -160,15 +156,15 @@ func (r *KserveGatewayReconciler) getExistingResource(ctx context.Context) (*ist
 func (r *KserveGatewayReconciler) Delete(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error {
 	var errs []error
 
-	log.V(1).Info(fmt.Sprintf("Deleting serving certificate Secret(%s) in %s namespace", destSecretName, isvc.Namespace))
-	if err := r.deleteServingCertSecretInIstioNamespace(ctx, destSecretName); err != nil {
-		log.V(1).Error(err, fmt.Sprintf("Failed to delete the copied serving certificate Secret(%s) in %s namespace", destSecretName, isvc.Namespace))
+	log.V(1).Info(fmt.Sprintf("Deleting serving certificate Secret(%s) in %s namespace", fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace), isvc.Namespace))
+	if err := r.deleteServingCertSecretInIstioNamespace(ctx, fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace)); err != nil {
+		log.V(1).Error(err, fmt.Sprintf("Failed to delete the copied serving certificate Secret(%s) in %s namespace", fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace), isvc.Namespace))
 		errs = append(errs, err)
 	}
 
-	log.V(1).Info(fmt.Sprintf("Deleting the Server(%s) from KServe local gateway in the %s namespace", portName, meshNamespace))
-	if err := r.removeServerFromGateway(ctx, log, portName); err != nil {
-		log.V(1).Error(err, fmt.Sprintf("Failed to remove the Server(%s) from KServe local gateway in the %s namespace", portName, meshNamespace))
+	log.V(1).Info(fmt.Sprintf("Deleting the Server(%s) from KServe local gateway in the %s namespace", fmt.Sprintf("%s-%s", "https", isvc.Name), meshNamespace))
+	if err := r.removeServerFromGateway(ctx, log, fmt.Sprintf("%s-%s", "https", isvc.Name)); err != nil {
+		log.V(1).Error(err, fmt.Sprintf("Failed to remove the Server(%s) from KServe local gateway in the %s namespace", fmt.Sprintf("%s-%s", "https", isvc.Name), meshNamespace))
 		errs = append(errs, err)
 	}
 
@@ -228,7 +224,7 @@ func (r *KserveGatewayReconciler) removeServerFromGateway(ctx context.Context, l
 func (r *KserveGatewayReconciler) copyServingCertSecretFromIsvcNamespace(ctx context.Context, sourceSecret *corev1.Secret, preDestSecret *corev1.Secret) error {
 	destinationSecret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      destSecretName,
+			Name:      fmt.Sprintf("%s-%s", sourceSecret.Name, sourceSecret.Namespace),
 			Namespace: meshNamespace,
 		},
 		Data: sourceSecret.Data,

controllers/webhook/kserve_service_mutator.go

@@ -4,9 +4,7 @@ import (
 	"context"
 	"fmt"
 
-	kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -27,28 +25,24 @@ func NewKserveServiceMutator(client client.Client) *kserveServiceMutator {
 // Default implements the admission.Defaulter interface to mutate the resources
 func (m *kserveServiceMutator) Default(ctx context.Context, obj runtime.Object) error {
 	log := logf.FromContext(ctx).WithName("KserviceServiceMutateWebhook")
-
+	log.Info("AAAAAAAAAAAAAAAAAAA - mutatingwebhook Called")
+
 	svc, ok := obj.(*corev1.Service)
 	if !ok {
 		return fmt.Errorf("unexpected object type")
 	}
-
-	// Retrieve the InferenceService directly by name
-	var isvc kservev1beta1.InferenceService
-	err := m.client.Get(ctx, client.ObjectKey{Name: svc.Name, Namespace: svc.Namespace}, &isvc)
-	if err != nil {
-		if apierrors.IsNotFound(err) {
-			// If InferenceService is not found, return without error
-			return nil
-		}
-		return fmt.Errorf("failed to get InferenceService: %w", err)
+
+	log.Info("AAAAAAAAAAAAAAAAAAA1 - before checking")
+	if !hasInferenceServiceOwner(svc) {
+		return nil
 	}
+	log.Info("AAAAAAAAAAAAAAAAAAA1 - after checking")
 
 	// Add the annotation if a matching InferenceService is found
 	if svc.Annotations == nil {
 		svc.Annotations = make(map[string]string)
 	}
-	svc.Annotations["service.beta.openshift.io/serving-cert-secret-name"] = isvc.Name
+	svc.Annotations["service.beta.openshift.io/serving-cert-secret-name"] = svc.Name
 	log.Info("Added annotation to Service", "ServiceName", svc.Name, "Annotation", svc.Annotations["service.beta.openshift.io/serving-cert-secret-name"])
 
 	return nil
@@ -59,3 +53,14 @@ func (m *kserveServiceMutator) InjectDecoder(d *admission.Decoder) error {
 	m.Decoder = d
 	return nil
 }
+
+// check if the src secret has ownerReferences for InferenceService
+func hasInferenceServiceOwner(obj client.Object) bool {
+	ownerReferences := obj.GetOwnerReferences()
+	for _, ownerReference := range ownerReferences {
+		if ownerReference.Kind == "InferenceService" {
+			return true
+		}
+	}
+	return false
+}

controllers/webhook_kserve_service_mutator_test.go

@@ -19,6 +19,13 @@ var _ = Describe("KServe Service mutator webhook", func() {
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      defaultIsvcName,
 				Namespace: "default",
+				OwnerReferences: []metav1.OwnerReference{
+					{
+						APIVersion: "serving.kserve.io/v1beta1",
+						Kind:       "InferenceService",
+						Name:       "sklearn-example-isvc-iris-v2-rest",
+					},
+				},
 			},
 		}
 	}
@@ -28,7 +35,7 @@ var _ = Describe("KServe Service mutator webhook", func() {
 
 	})
 
-	It("adds serving cert annotation if Service name matches InferenceService name", func() {
+	It("adds serving cert annotation when Service have InferenceService ownerReference", func() {
 		// Create a new InferenceService
 		inferenceService := &kservev1beta1.InferenceService{
 			ObjectMeta: metav1.ObjectMeta{
@@ -48,10 +55,11 @@ var _ = Describe("KServe Service mutator webhook", func() {
 		Expect(kserveService.Annotations["service.beta.openshift.io/serving-cert-secret-name"]).To(Equal(defaultIsvcName))
 	})
 
-	It("skips adding annotation when Service name does not match InferenceService name", func() {
+	It("skips adding annotation when Service does not have InferenceService ownerReference", func() {
 		kserveServiceName := "different-name"
 		kserveService := createServiceOwnedKserve()
 		kserveService.SetName(kserveServiceName)
+		kserveService.SetOwnerReferences([]metav1.OwnerReference{})
 
 		err := mutator.Default(ctx, kserveService)
 		Expect(err).ShouldNot(HaveOccurred())

go.mod

@@ -7,7 +7,6 @@ require (
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/kserve/kserve v0.12.1
 	github.com/kuadrant/authorino v0.15.0
-	github.com/moby/moby v27.0.0+incompatible
 	github.com/onsi/ginkgo v1.16.5
 	github.com/onsi/gomega v1.30.0
 	github.com/opendatahub-io/model-registry v0.1.1
@@ -40,14 +39,7 @@ require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/blendle/zapdriver v1.3.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
-	github.com/containerd/fifo v1.1.0 // indirect
-	github.com/containerd/log v0.1.0 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
-	github.com/docker/distribution v2.8.2+incompatible // indirect
-	github.com/docker/docker v24.0.7+incompatible // indirect
-	github.com/docker/go-connections v0.4.0 // indirect
-	github.com/docker/go-metrics v0.0.1 // indirect
-	github.com/docker/go-units v0.5.0 // indirect
 	github.com/emicklei/go-restful/v3 v3.11.0 // indirect
 	github.com/evanphx/json-patch/v5 v5.7.0 // indirect
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
@@ -75,21 +67,16 @@ require (
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect
-	github.com/moby/term v0.5.0 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
-	github.com/morikuni/aec v1.0.0 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/nxadm/tail v1.4.8 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect
-	github.com/opencontainers/image-spec v1.1.0-rc5 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/prometheus/client_golang v1.17.0 // indirect
 	github.com/prometheus/client_model v0.5.0 // indirect
 	github.com/prometheus/common v0.45.0 // indirect
 	github.com/prometheus/procfs v0.12.0 // indirect
-	github.com/rootless-containers/rootlesskit v1.1.1 // indirect
-	github.com/sirupsen/logrus v1.9.3 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/tidwall/match v1.1.1 // indirect
 	github.com/tidwall/pretty v1.2.1 // indirect