fix: extend profile AuthorizationPolicy for KServe models #6627
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: AlexandreBrown The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
You should rather work on the proper upstream issue to solve this once and for all. I prefer security by default such that the user has to explictly disable the security. Also hardening via #6013 is worth a look There are three possibilities until the upstream stuff is properly fixed
apiVersion: security.istio.io/v1beta1
kind: AuthorizationPolicy
metadata:
name: allow-in-cluster-kserve
spec:
rules:
- to:
- operation:
paths:
- /v1/models/*
- /v2/models/*
All three of them can be implemented with simple kyverno clusterpolicies if you want to have them by default. So i favor not merging this and putting more pressure on upstream. |
| @@ -442,6 +442,9 @@ func (r *ProfileReconciler) getAuthorizationPolicy(profileIns *profilev1.Profile | |||
| "/healthz", | |||
| "/metrics", | |||
| "/wait-for-drain", | |||
| "/ready", | |||
| "/v1/models/*", | |||
There was a problem hiding this comment.
@surajkota allowing paths does not lock down the traffic from other namespaces. I think we still want the changes in https://github.com/kubeflow/kubeflow/pull/6013/files for only allowing requests from knative-serving namespace and in addition we need to lock down the original namespace of the request, unfortunately I couldn't find a header to check the original namespace when the requests are proxyed by activator.
There was a problem hiding this comment.
Maybe we should rely on kubeflow-userid header?
|
@surajkota What's the plan for this PR? |
|
We need to update/replace this PR with one that has a dynamic principal (so we don't assume the ServiceAccount or Namespace of the KServe), for example, this PR did it for Notebooks/Pipelines: #7310 NOTE: this version of the PR does not actually restrict by source principal (but it should), see the original one for more context: #6013 |
thesuperzapper
changed the title
Workaround for: kubeflow/dashboard#13, kserve/kserve#1558
KServe integration with profiles is not complete. Users can create an inferenceservice but cannot use them(i.e. get prediction requests served) from inside or outside the cluster without doing one of the following(according to kserve/kserve#1558):
This PR automates the generation of the required authorization policy in profile namespace but this also means that a user can get data from these paths from other user namespaces.
@kimwnasptd @yuzisun Question: Can we add these paths to profile's authorization policy and call out the limitation with multi user isolation in docs until kserve/kserve#1371 is resolved? Note: profile namespaces already allow querying
/healthz,/metricsand/wait-for-drainpaths.Current multi user isolation limitations are listed here: https://www.kubeflow.org/docs/components/pipelines/overview/multi-user/#current-limitations
Testing: