-
Notifications
You must be signed in to change notification settings - Fork 55
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #181 from israel-hdez/jira-2542-fix
Prevent creation of KSVC if its namespace is not in the Mesh
- Loading branch information
Showing
13 changed files
with
360 additions
and
11 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| bases: | ||
| - ../rbac | ||
| - ../manager | ||
| - ../webhook |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| apiVersion: kustomize.config.k8s.io/v1beta1 | ||
| kind: Kustomization | ||
|
|
||
| resources: | ||
| - manifests.yaml | ||
| - service.yaml |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| --- | ||
| apiVersion: admissionregistration.k8s.io/v1 | ||
| kind: ValidatingWebhookConfiguration | ||
| metadata: | ||
| name: validating.odh-model-controller.opendatahub.io | ||
| annotations: | ||
| service.beta.openshift.io/inject-cabundle: true | ||
| webhooks: | ||
| - admissionReviewVersions: | ||
| - v1 | ||
| clientConfig: | ||
| service: | ||
| name: odh-model-controller-webhook-service | ||
| path: /validate-serving-knative-dev-v1-service | ||
| failurePolicy: Fail | ||
| name: validating.ksvc.odh-model-controller.opendatahub.io | ||
| rules: | ||
| - apiGroups: | ||
| - serving.knative.dev | ||
| apiVersions: | ||
| - v1 | ||
| operations: | ||
| - CREATE | ||
| resources: | ||
| - services | ||
| sideEffects: None |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: odh-model-controller-webhook-service | ||
| annotations: | ||
| service.beta.openshift.io/serving-cert-secret-name: odh-model-controller-webhook-cert | ||
| spec: | ||
| ports: | ||
| - port: 443 | ||
| targetPort: webhook-server | ||
| selector: | ||
| control-plane: odh-model-controller |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| package webhook | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "strings" | ||
|
|
||
| kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| types2 "k8s.io/apimachinery/pkg/types" | ||
| knservingv1 "knative.dev/serving/pkg/apis/serving/v1" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| logf "sigs.k8s.io/controller-runtime/pkg/log" | ||
|
|
||
| "github.com/opendatahub-io/odh-model-controller/controllers/constants" | ||
| "github.com/opendatahub-io/odh-model-controller/controllers/resources" | ||
| ) | ||
|
|
||
| // +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-serving-knative-dev-v1-service,mutating=false,failurePolicy=fail,groups="serving.knative.dev",resources=services,verbs=create,versions=v1,name=validating.ksvc.odh-model-controller.opendatahub.io,sideEffects=None | ||
|
|
||
| type ksvcValidator struct { | ||
| client client.Client | ||
| } | ||
|
|
||
| func NewKsvcValidator(client client.Client) *ksvcValidator { | ||
| return &ksvcValidator{client: client} | ||
| } | ||
|
|
||
| func (v *ksvcValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error { | ||
| log := logf.FromContext(ctx).WithName("KsvcValidatingWebhook") | ||
| ksvc, ok := obj.(*knservingv1.Service) | ||
| if !ok { | ||
| return fmt.Errorf("expected a Knative Service but got a %T", obj) | ||
| } | ||
|
|
||
| log = log.WithValues("namespace", ksvc.Namespace, "ksvc", ksvc.Name) | ||
| log.Info("Validating Knative Service") | ||
|
|
||
| // If there is an explicit intent for not having a sidecar, skip validation | ||
| ksvcTemplateMeta := ksvc.Spec.Template.GetObjectMeta() | ||
| if ksvcTemplateMeta != nil { | ||
| if templateAnnotations := ksvcTemplateMeta.GetAnnotations(); templateAnnotations[constants.IstioSidecarInjectAnnotationName] == "false" { | ||
| log.V(1).Info("Skipping validation of Knative Service because there is an explicit intent to exclude it from the Service Mesh") | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| // Only validate the KSVC if it is owned by KServe controller | ||
| ksvcMetadata := ksvc.GetObjectMeta() | ||
| if ksvcMetadata == nil { | ||
| log.V(1).Info("Skipping validation of Knative Service because it does not have metadata") | ||
| return nil | ||
| } | ||
| ksvcOwnerReferences := ksvcMetadata.GetOwnerReferences() | ||
| if ksvcOwnerReferences == nil { | ||
| log.V(1).Info("Skipping validation of Knative Service because it does not have owner references") | ||
| return nil | ||
| } | ||
| isOwnedByKServe := false | ||
| for _, owner := range ksvcOwnerReferences { | ||
| if owner.Kind == constants.InferenceServiceKind { | ||
| if strings.Contains(owner.APIVersion, kservev1beta1.SchemeGroupVersion.Group) { | ||
| isOwnedByKServe = true | ||
| } | ||
| } | ||
| } | ||
| if !isOwnedByKServe { | ||
| log.V(1).Info("Skipping validation of Knative Service because it is not owned by KServe") | ||
| return nil | ||
| } | ||
|
|
||
| // Since the Ksvc is owned by an InferenceService, it is known that it is required to be | ||
| // in the Mesh. Thus, the involved namespace needs to be enrolled in the mesh. | ||
| // Go and check the ServiceMeshMemberRoll to verify that the namespace is already a | ||
| // member. If it is still not a member, reject creation of the Ksvc to prevent | ||
| // creation of a Pod that would not be in the Mesh. | ||
| smmrQuerier := resources.NewServiceMeshMemberRole(v.client) | ||
| smmr, fetchSmmrErr := smmrQuerier.FetchSMMR(ctx, log, types2.NamespacedName{Name: constants.ServiceMeshMemberRollName, Namespace: constants.IstioNamespace}) | ||
| if fetchSmmrErr != nil { | ||
| log.Error(fetchSmmrErr, "Error when fetching ServiceMeshMemberRoll", "smmr.namespace", constants.IstioNamespace, "smmr.name", constants.ServiceMeshMemberRollName) | ||
| return fetchSmmrErr | ||
| } | ||
| if smmr == nil { | ||
| log.Info("Rejecting Knative service because the ServiceMeshMemberRoll does not exist", "smmr.namespace", constants.IstioNamespace, "smmr.name", constants.ServiceMeshMemberRollName) | ||
| return fmt.Errorf("rejecting creation of Knative service %s on namespace %s because the ServiceMeshMemberRoll does not exist", ksvc.Name, ksvc.Namespace) | ||
| } | ||
|
|
||
| log = log.WithValues("smmr.namespace", smmr.Namespace, "smmr.name", smmr.Name) | ||
|
|
||
| for _, memberNamespace := range smmr.Status.ConfiguredMembers { | ||
| if memberNamespace == ksvc.Namespace { | ||
| log.V(1).Info("The Knative service is accepted") | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| log.Info("Rejecting Knative service because its namespace is not a member of the service mesh") | ||
| return fmt.Errorf("rejecting creation of Knative service %s on namespace %s because the namespace is not a configured member of the mesh", ksvc.Name, ksvc.Namespace) | ||
| } | ||
|
|
||
| func (v *ksvcValidator) ValidateUpdate(_ context.Context, _, _ runtime.Object) error { | ||
| // Nothing to validate on updates | ||
| return nil | ||
| } | ||
|
|
||
| func (v *ksvcValidator) ValidateDelete(_ context.Context, _ runtime.Object) error { | ||
| // For deletion, we don't need to validate anything. | ||
| return nil | ||
| } |
Oops, something went wrong.