kubeflow icon indicating copy to clipboard operation
kubeflow copied to clipboard

profile-controller: Limit access to serving paths to KNative sa

Open davidspek opened this issue 4 years ago • 4 comments
trafficstars

In https://github.com/kubeflow/kubeflow/pull/5848 the profile controller was updated to allow access to paths in user namespaces that KNative requires for launching workloads. However, the implementation allows all access to the /healthz, /metrics and ``/wait-for-drain paths in a user's namespace. This means that a user can get data from these (commonly used) paths from all other user namespaces, breaking multi-user isolation. This PR fixes this by only allowing the service account used by KNative access to these paths in user namespaces. It also adds 2 more paths, /ready which is used by KNative, and /v1/models/* which is used by KFServing as outlined in this comment.

/cc @kubeflow/wg-notebooks-leads @yanniszark @kubeflow/wg-serving-leads

davidspek avatar Jun 23 '21 17:06 davidspek

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please assign stefanofioravanzo after the PR has been reviewed. You can assign the PR to them by writing /assign @stefanofioravanzo in a comment when ready.

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 /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

google-oss-robot avatar Jun 23 '21 17:06 google-oss-robot

CI is failing because the PR needs to be rebased, but I am delaying doing this since it means the deployment manifests for the profile controller will need to be changed because of https://github.com/kubeflow/kubeflow/pull/5761. Once this PR is closer to being in a final state I will rebase it.

davidspek avatar Jun 23 '21 20:06 davidspek

Thanks for the fix @DavidSpek! It worked for me, but it seems like we also need to add "/v2/models/*" to the list to support predictor protocol v2 of KServe.

alembiewski avatar Dec 11 '21 11:12 alembiewski

This issue has been automatically marked as stale because it has not had activity in 90 days. It will be closed in 7 days if no further activity occurs.

Thank you for your contributions.


Issues never become stale if any of the following is true:

  1. they are added to a GitHub Project
  2. they are added to a GitHub Milestone
  3. they have a priority label: priority/p0, priority/p1, priority/p2, priority/p3
  4. they have the frozen label: lifecycle/frozen

stale[bot] avatar Sep 21 '22 05:09 stale[bot]

Can you join the next notebooks WG meeting in two weeks ? We are always looking for help.

https://www.kubeflow.org/docs/about/community/#kubeflow-community-calendars has the calendar and meeting notes.

You can also reach us on slack in the channel wg-notebooks and wg-manifests.

juliusvonkohout avatar Aug 31 '23 16:08 juliusvonkohout

Let's discuss this more on this PR: https://github.com/kubeflow/kubeflow/pull/6627

thesuperzapper avatar May 12 '24 00:05 thesuperzapper