Skip to content

Commit

Permalink
Create webhook for NIM enablement
Browse files Browse the repository at this point in the history
Signed-off-by: Xieshen Zhang <[email protected]>
  • Loading branch information
xieshenzh committed Nov 6, 2024
1 parent 18a5e80 commit 09288d1
Show file tree
Hide file tree
Showing 8 changed files with 529 additions and 25 deletions.
5 changes: 4 additions & 1 deletion PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ resources:
group: nim
kind: Account
path: github.com/opendatahub-io/odh-model-controller/api/nim/v1
version: v1
version: v1
webhooks:
validation: true
webhookVersion: v1
20 changes: 20 additions & 0 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,23 @@ webhooks:
resources:
- services
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
service:
name: webhook-service
namespace: system
path: /validate-nim-opendatahub-io-v1-account
failurePolicy: Fail
name: validating.nim.account.odh-model-controller.opendatahub.io
rules:
- apiGroups:
- nim.opendatahub.io
apiVersions:
- v1
operations:
- CREATE
- UPDATE
resources:
- accounts
sideEffects: None
4 changes: 4 additions & 0 deletions config/webhook/webhook_patch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ webhooks:
matchExpressions:
- key: serving.kserve.io/inferenceservice
operator: Exists
- name: validating.nim.account.odh-model-controller.opendatahub.io
clientConfig:
service:
name: odh-model-controller-webhook-service
3 changes: 2 additions & 1 deletion controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ var _ = BeforeSuite(func() {
By("Bootstrapping test environment")
envTest = &envtest.Environment{
CRDInstallOptions: envtest.CRDInstallOptions{
Paths: []string{filepath.Join("..", "config", "crd", "external")},
Paths: []string{filepath.Join("..", "config", "crd", "external"),
filepath.Join("..", "config", "crd", "bases")},
ErrorIfPathMissing: true,
CleanUpAfterUse: false,
},
Expand Down
54 changes: 31 additions & 23 deletions controllers/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const (
inferenceServiceDeploymentModeAnnotation = "serving.kserve.io/deploymentMode"
KserveConfigMapName = "inferenceservice-config"
KServeWithServiceMeshComponent = "kserve-service-mesh"
KServeWithNIMComponent = "kserve-nim"
)

func GetDeploymentModeForIsvc(ctx context.Context, cli client.Client, isvc *kservev1beta1.InferenceService) (IsvcDeploymentMode, error) {
Expand Down Expand Up @@ -103,39 +104,46 @@ func VerifyIfComponentIsEnabled(ctx context.Context, cli client.Client, componen
// there must be only one dsc
if len(objectList.Items) == 1 {
fields := []string{"spec", "components", componentName, "managementState"}
if componentName == KServeWithServiceMeshComponent {
switch componentName {
case KServeWithServiceMeshComponent:
// For KServe, Authorino is required when serving is enabled
// By Disabling ServiceMesh for RawDeployment, it should reflect on disabling
// the Authorino integration as well.
fields = []string{"spec", "components", "kserve", "serving", "managementState"}
kserveFields := []string{"spec", "components", "kserve", "managementState"}

serving, _, errServing := unstructured.NestedString(objectList.Items[0].Object, fields...)
if errServing != nil {
return false, fmt.Errorf("failed to retrieve the component [%s] status from %+v. %w",
componentName, objectList.Items[0], errServing)
}

kserve, _, errKserve := unstructured.NestedString(objectList.Items[0].Object, kserveFields...)
if errKserve != nil {
return false, fmt.Errorf("failed to retrieve the component [%s] status from %+v. %w",
componentName, objectList.Items[0], errKserve)
return checkIfKServeComponentIsEnabled(objectList.Items[0], "serving", componentName)
case KServeWithNIMComponent:
return checkIfKServeComponentIsEnabled(objectList.Items[0], "nim", componentName)
default:
val, _, err := unstructured.NestedString(objectList.Items[0].Object, fields...)
if err != nil {
return false, fmt.Errorf("failed to retrieve the component [%s] status from %+v",
componentName, objectList.Items[0])
}

return (serving == "Managed" || serving == "Unmanaged") && kserve == "Managed", nil
}

val, _, err := unstructured.NestedString(objectList.Items[0].Object, fields...)
if err != nil {
return false, fmt.Errorf("failed to retrieve the component [%s] status from %+v",
componentName, objectList.Items[0])
return val == "Managed", nil
}
return val == "Managed", nil
} else {
return false, fmt.Errorf("there is no %s available in the cluster", GVK.DataScienceCluster.Kind)
}
}

func checkIfKServeComponentIsEnabled(obj unstructured.Unstructured, component string, componentName string) (bool, error) {
fields := []string{"spec", "components", "kserve", component, "managementState"}
kserveFields := []string{"spec", "components", "kserve", "managementState"}

comp, _, errComp := unstructured.NestedString(obj.Object, fields...)
if errComp != nil {
return false, fmt.Errorf("failed to retrieve the component [%s] status from %+v. %w",
componentName, obj, errComp)
}

kserve, _, errKserve := unstructured.NestedString(obj.Object, kserveFields...)
if errKserve != nil {
return false, fmt.Errorf("failed to retrieve the component [%s] status from %+v. %w",
componentName, obj, errKserve)
}

return (comp == "Managed" || comp == "Unmanaged") && kserve == "Managed", nil
}

// AuthorinoEnabledWhenOperatorNotMissing is a helper function to check if Authorino is enabled when the Operator is not missing.
// This is defined by Condition.Reason=MissingOperator. In any other case, it is assumed that Authorino is enabled but DSC might not be
// in the Ready state and will reconcile.
Expand Down
118 changes: 118 additions & 0 deletions controllers/webhook/nim_account_webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
package webhook

import (
"context"
"fmt"
"reflect"

nimv1 "github.com/opendatahub-io/odh-model-controller/api/nim/v1"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-nim-opendatahub-io-v1-account,mutating=false,failurePolicy=fail,groups=nim.opendatahub.io,resources=accounts,verbs=create;update,versions=v1,name=validating.nim.account.odh-model-controller.opendatahub.io,sideEffects=None

type nimAccountValidator struct {
client client.Client
}

func NewNimAccountValidator(client client.Client) admission.CustomValidator {
return &nimAccountValidator{client: client}
}

func (v *nimAccountValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) {
log := logf.FromContext(ctx).WithName("NIMAccountValidatingWebhook")

account := obj.(*nimv1.Account)

log = log.WithValues("namespace", account.Namespace, "account", account.Name)
log.Info("Validating NIM Account creation")

err = v.verifySingletonInNamespace(ctx, account)
if err != nil {
log.Error(err, "Rejecting NIM Account creation because checking singleton didn't pass")
return nil, err
}

err = v.verifyAPIKeySecret(ctx, nil, account)
if err != nil {
log.Error(err, "Rejecting NIM Account creation because checking API key Secret didn't pass")
return nil, err
}

return nil, nil
}

func (v *nimAccountValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) {
log := logf.FromContext(ctx).WithName("NIMAccountValidatingWebhook")

account := newObj.(*nimv1.Account)

log = log.WithValues("namespace", account.Namespace, "account", account.Name)
log.Info("Validating NIM Account update")

err = v.verifyAPIKeySecret(ctx, oldObj.(*nimv1.Account), account)
if err != nil {
log.Error(err, "Rejecting NIM Account update because checking API key Secret didn't pass")
return nil, err
}

return nil, nil
}

func (v *nimAccountValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) {
// For deletion, nothing needs to be validated
return nil, nil
}

func (v *nimAccountValidator) verifyAPIKeySecret(ctx context.Context, oldAccount, newAccount *nimv1.Account) error {
if oldAccount == nil || !reflect.DeepEqual(oldAccount.Spec.APIKeySecret, newAccount.Spec.APIKeySecret) {
if newAccount.Spec.APIKeySecret.Name == "" {
msg := "name of the Secret for the API key is invalid"
return field.Invalid(field.NewPath("spec").Child("apiKeySecret").Child("name"), newAccount.Spec.APIKeySecret.Name, msg)
}
name := newAccount.Spec.APIKeySecret.Name
namespace := newAccount.Spec.APIKeySecret.Namespace
if namespace == "" {
namespace = newAccount.Namespace
}

err := v.client.Get(ctx,
types.NamespacedName{
Name: name,
Namespace: namespace,
},
&v1.Secret{})
if err != nil {
if errors.IsNotFound(err) {
msg := "Secret for the API key cannot be found"
return field.Invalid(field.NewPath("spec").Child("apiKeySecret").Child("name"), newAccount.Spec.APIKeySecret.Name, msg)
} else {
return fmt.Errorf("failed to verify the API key Secret with err: %s", err.Error())
}
}
}
return nil
}

func (v *nimAccountValidator) verifySingletonInNamespace(ctx context.Context, account *nimv1.Account) error {
accountList := nimv1.AccountList{}
err := v.client.List(ctx,
&accountList,
client.InNamespace(account.Namespace))
if err != nil {
return fmt.Errorf("failed to verify if there are existing Accounts with err: %s", err.Error())
}

if len(accountList.Items) > 0 {
return fmt.Errorf("rejecting creation of Account %s in namespace %s because there is already an Account created in the namespace", account.Name, account.Namespace)
}
return nil
}
Loading

0 comments on commit 09288d1

Please sign in to comment.