[RHOAIENG-13638] - Do not allow isvc creation in protected isvc (open…

…datahub-io#311)

* [RHOAIENG-13638] - Do not allow isvc creation in protected namespace

chore: Fixes [RHOAIENG-13638] - Kserve model is not Ready after a kserve model is created and deleted from istio-system namespace

Signed-off-by: Spolti <fspolti@redhat.com>

* review suggestions

Signed-off-by: Spolti <fspolti@redhat.com>

* Update controllers/webhook/isvc_validator.go

Co-authored-by: Edgar Hernández <ehernand@redhat.com>
Signed-off-by: Spolti <fspolti@redhat.com>

---------

Signed-off-by: Spolti <fspolti@redhat.com>
Co-authored-by: Edgar Hernández <ehernand@redhat.com>

config/base/params.env

@@ -3,4 +3,4 @@ caikit-tgis-image=quay.io/opendatahub/caikit-tgis-serving:stable-2d9db23
 caikit-standalone-image=quay.io/opendatahub/caikit-nlp:stable-994ec60
 tgis-image=quay.io/opendatahub/text-generation-inference:stable-eba83ba
 ovms-image=quay.io/opendatahub/openvino_model_server:2024.3-release-4c8c52c	
-vllm-image=quay.io/opendatahub/vllm:stable-849f0f5
+vllm-image=quay.io/opendatahub/vllm:stable-849f0f5

config/webhook/manifests.yaml

@@ -4,6 +4,25 @@ kind: ValidatingWebhookConfiguration
 metadata:
   name: validating-webhook-configuration
 webhooks:
+- admissionReviewVersions:
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-isvc-odh-service
+  failurePolicy: Fail
+  name: validating.isvc.odh-model-controller.opendatahub.io
+  rules:
+  - apiGroups:
+    - serving.kserve.io
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    resources:
+    - inferenceservices
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:

config/webhook/webhook_patch.yaml

@@ -17,3 +17,7 @@ webhooks:
     clientConfig:
       service:
         name: odh-model-controller-webhook-service
+  - name: validating.isvc.odh-model-controller.opendatahub.io
+    clientConfig:
+      service:
+        name: odh-model-controller-webhook-service

controllers/kserve_inferenceservice_controller_metrics_test.go

@@ -63,7 +63,6 @@ var _ = Describe("The KServe Dashboard reconciler", func() {
 		if err := cli.Create(ctx, inferenceServiceConfig); err != nil && !errors.IsAlreadyExists(err) {
 			Fail(err.Error())
 		}
-
 	})
 
 	When("deploying a Kserve model", func() {

controllers/storageconfig_controller_test.go

@@ -183,7 +183,7 @@ var _ = Describe("StorageConfig controller", func() {
 			expectedUpdatedStorageConfigSecret := &corev1.Secret{}
 			err = convertToStructuredResource(storageconfigUpdatedCertEncodedPath, expectedUpdatedStorageConfigSecret)
 			Expect(err).NotTo(HaveOccurred())
-			
+
 			Expect(compareSecrets(updatedStorageconfigSecret, expectedUpdatedStorageConfigSecret)).Should((BeTrue()))
 		})
 	})

controllers/utils/utils.go

@@ -4,15 +4,16 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"github.com/go-logr/logr"
-	kservev1alpha1 "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
-	"github.com/pkg/errors"
-	"k8s.io/apimachinery/pkg/types"
 	"os"
 	"reflect"
 	"sort"
 	"strings"
 
+	"github.com/go-logr/logr"
+	kservev1alpha1 "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
+	"github.com/pkg/errors"
+	"k8s.io/apimachinery/pkg/types"
+
 	kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
 	"github.com/kuadrant/authorino/pkg/log"
 	"github.com/opendatahub-io/odh-model-controller/controllers/constants"
@@ -37,6 +38,8 @@ var (
 	ModelMesh     IsvcDeploymentMode = "ModelMesh"
 
 	gvResourcesCache map[string]*metav1.APIResourceList
+
+	_appNamespace *string
 )
 
 const (
@@ -264,6 +267,30 @@ func VerifyIfMeshAuthorizationIsEnabled(ctx context.Context, cli client.Client)
 	return false, nil
 }
 
+// GetApplicationNamespace returns the namespace where the application components are installed.
+// defaults to: RHOAI - redhat-ods-applications, ODH: opendatahub
+func GetApplicationNamespace(ctx context.Context, cli client.Client) (string, error) {
+	if _appNamespace != nil {
+		return *_appNamespace, nil
+	}
+
+	podNamespace := os.Getenv("POD_NAMESPACE")
+	objectList, err := getDSCIObject(ctx, cli)
+	if err != nil {
+		log.V(0).Error(err, "Failed to fetch the DSCI object")
+		return "", err
+	}
+
+	for _, item := range objectList.Items {
+		ns, _, _ := unstructured.NestedString(item.Object, "spec", "applicationsNamespace")
+		if len(ns) > 0 {
+			podNamespace = ns
+		}
+	}
+	_appNamespace = &podNamespace
+	return podNamespace, nil
+}
+
 func IsNil(i any) bool {
 	return reflect.ValueOf(i).IsNil()
 }

controllers/webhook/isvc_validator.go

@@ -0,0 +1,84 @@
+/*
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package webhook
+
+import (
+	"context"
+	"fmt"
+	kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
+	"github.com/opendatahub-io/odh-model-controller/controllers/utils"
+	"net/http"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+)
+
+// +kubebuilder:webhook:admissionReviewVersions=v1beta1,path=/validate-isvc-odh-service,mutating=false,failurePolicy=fail,groups="serving.kserve.io",resources=inferenceservices,verbs=create,versions=v1beta1,name=validating.isvc.odh-model-controller.opendatahub.io,sideEffects=None
+
+type IsvcValidator struct {
+	Client  client.Client
+	Decoder *admission.Decoder
+}
+
+// NewIsvcValidator For tests purposes
+func NewIsvcValidator(client client.Client, decoder *admission.Decoder) *IsvcValidator {
+	return &IsvcValidator{
+		Client:  client,
+		Decoder: decoder,
+	}
+}
+
+// Handle implements admission.Validator so a webhook will be registered for the type serving.kserve.io.inferenceServices
+// This webhook will filter out the protected namespaces preventing the user to create the InferenceService in those namespaces
+// which are: The Istio control plane namespace, Application namespace and Serving namespace
+// func (is *isvcValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) {
+func (is *IsvcValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
+	protectedNamespaces := make([]string, 3)
+	// hardcoding for now since there is no plan to install knative on other namespaces
+	protectedNamespaces[0] = "knative-serving"
+
+	log := logf.FromContext(ctx).WithName("InferenceServiceValidatingWebhook")
+
+	isvc := &kservev1beta1.InferenceService{}
+	errs := is.Decoder.Decode(req, isvc)
+	if errs != nil {
+		return admission.Errored(http.StatusBadRequest, errs)
+	}
+
+	log = log.WithValues("namespace", isvc.Namespace, "isvc", isvc.Name)
+	log.Info("Validating InferenceService")
+
+	_, meshNamespace := utils.GetIstioControlPlaneName(ctx, is.Client)
+	protectedNamespaces[1] = meshNamespace
+
+	appNamespace, err := utils.GetApplicationNamespace(ctx, is.Client)
+	if err != nil {
+		return admission.Errored(http.StatusBadRequest, err)
+	}
+	protectedNamespaces[2] = appNamespace
+
+	log.Info("Filtering protected namespaces", "namespaces", protectedNamespaces)
+	for _, ns := range protectedNamespaces {
+		if isvc.Namespace == ns {
+			log.V(1).Info("Namespace is protected, the InferenceService will not be created")
+			return admission.Denied(fmt.Sprintf("The InferenceService %s "+
+				"cannot be created in protected namespace %s.", isvc.Name, isvc.Namespace))
+		}
+	}
+	log.Info("Namespace is not protected")
+	return admission.Allowed("Namespace is not protected")
+}

controllers/webhook/ksvc_validator.go

@@ -1,3 +1,18 @@
+/*
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package webhook
 
 import (

controllers/webhook/nim_account_webhook.go

@@ -1,3 +1,18 @@
+/*
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package webhook
 
 import (

controllers/webhook_isvc_validator_test.go

@@ -0,0 +1,108 @@
+/*
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controllers
+
+import (
+	"encoding/json"
+	kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/opendatahub-io/odh-model-controller/controllers/utils"
+	"github.com/opendatahub-io/odh-model-controller/controllers/webhook"
+	admissionv1 "k8s.io/api/admission/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+var _ = Describe("Inference Service validator webhook", func() {
+	var validator *webhook.IsvcValidator
+	var meshNamespace, appsNamespace string
+
+	createInferenceService := func(namespace, name string) *kservev1beta1.InferenceService {
+		inferenceService := &kservev1beta1.InferenceService{}
+		err := convertToStructuredResource(KserveInferenceServicePath1, inferenceService)
+		Expect(err).NotTo(HaveOccurred())
+		inferenceService.SetNamespace(namespace)
+		if len(name) != 0 {
+			inferenceService.Name = name
+		}
+		Expect(cli.Create(ctx, inferenceService)).Should(Succeed())
+		return inferenceService
+	}
+
+	BeforeEach(func() {
+		_, meshNamespace = utils.GetIstioControlPlaneName(ctx, cli)
+		appsNamespace, _ = utils.GetApplicationNamespace(ctx, cli)
+		validator = webhook.NewIsvcValidator(cli, admission.NewDecoder(cli.Scheme()))
+	})
+
+	It("Should allow the Inference Service in the test-model namespace", func() {
+		testNs := &corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-model",
+				Namespace: "test-model",
+			},
+		}
+		Expect(cli.Create(ctx, testNs)).Should(Succeed())
+
+		isvc := createInferenceService(testNs.Name, "test-isvc")
+		isvcBytes, err := json.Marshal(isvc)
+		Expect(err).NotTo(HaveOccurred())
+		response := validator.Handle(ctx, admission.Request{
+			AdmissionRequest: admissionv1.AdmissionRequest{Namespace: testNs.Name, Name: "test-isvc",
+				Object: runtime.RawExtension{Raw: isvcBytes}}})
+		Expect(response.Allowed).To(BeTrue())
+	})
+
+	It("Should not allow the Inference Service in the ServiceMesh namespace", func() {
+		isvc := createInferenceService(meshNamespace, "test-isvc")
+		isvcBytes, err := json.Marshal(isvc)
+		Expect(err).NotTo(HaveOccurred())
+		response := validator.Handle(ctx, admission.Request{
+			AdmissionRequest: admissionv1.AdmissionRequest{Namespace: meshNamespace, Name: "test-isvc",
+				Object: runtime.RawExtension{Raw: isvcBytes}}})
+		Expect(response.Allowed).To(BeFalse())
+	})
+
+	It("Should not allow the Inference Service in the ApplicationsNamespace namespace", func() {
+		isvc := createInferenceService(appsNamespace, "test-isvc")
+		isvcBytes, err := json.Marshal(isvc)
+		Expect(err).NotTo(HaveOccurred())
+		response := validator.Handle(ctx, admission.Request{
+			AdmissionRequest: admissionv1.AdmissionRequest{Namespace: appsNamespace, Name: "test-isvc",
+				Object: runtime.RawExtension{Raw: isvcBytes}}})
+		Expect(response.Allowed).To(BeFalse())
+	})
+
+	It("Should not allow the Inference Service in the knative-serving namespace", func() {
+		testNs := &corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "knative-serving",
+				Namespace: "knative-serving",
+			},
+		}
+		Expect(cli.Create(ctx, testNs)).Should(Succeed())
+		isvc := createInferenceService(testNs.Name, "test-isvc")
+		isvcBytes, err := json.Marshal(isvc)
+		Expect(err).NotTo(HaveOccurred())
+		response := validator.Handle(ctx, admission.Request{
+			AdmissionRequest: admissionv1.AdmissionRequest{Namespace: testNs.Name, Name: "test-isvc",
+				Object: runtime.RawExtension{Raw: isvcBytes}}})
+		Expect(response.Allowed).To(BeFalse())
+	})
+})

controllers/webhook_ksvc_validator_test.go

@@ -1,3 +1,18 @@
+/*
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package controllers
 
 import (