Adding groups support from request #2145
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: axel7083 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 |
google-oss-prow
bot
requested review from
anencore94,
johnugeorge and
kimwnasptd
|
hey @tenzen-y, IIRC KF did not support user groups back then. |
|
The Go Test / Unit Test (1.25.0) (pull_request) which failed surprise me a bit, I tried to reproduce it in local but got the following for the failing test $ go mod download && make test ENVTEST_K8S_VERSION=1.25.0
[...]
ok github.com/kubeflow/katib/pkg/controller.v1beta1/experiment 6.842s coverage: 76.7% of statements
[...] |
|
@axel7083 Yeah, that tests are flaky. I will restart it |
| user := r.Header.Get(USER_HEADER) | ||
| user = strings.Replace(user, USER_PREFIX, "", 1) | ||
|
|
||
| var groups []string | ||
| if header := r.Header.Get(GROUPS_HEADER); header != "" { | ||
| groups = strings.Split(header, ",") |
There was a problem hiding this comment.
Uhm. Probably, we should mention using , as a separator in the documentation.
@andreyvelich @johnugeorge Where should we notify users about this?
There was a problem hiding this comment.
I don't think we have any documentation regarding the Katib UI auth. I think, we should create separate page for Katib UI, and move all relative docs there. For example, move this one: https://www.kubeflow.org/docs/components/katib/hyperparameter/#accessing-the-katib-ui.
@apo-ger @kimwnasptd @elenzio9 Do we have any place in the docs where we explain users how to use Kubeflow UIs ?
There was a problem hiding this comment.
Sounds great.
Also, adding more documentation for the Katib UI auth there would be better. Through I see the section, we don't mention the auth.
There was a problem hiding this comment.
@tenzen-y do you want me to include in this PR a section in the accessing-the-katib-ui about the auth system ?
There was a problem hiding this comment.
@axel7083 Let's wait until @apo-ger, @elenzio9 , or @kimwnasptd comment.
They were the main contributors to support authzn in Katib UI.
There was a problem hiding this comment.
I meant we'd like to work on that at https://github.com/kubeflow/website.
|
Hi @axel7083, do we have enough time to finish this PR before our feature freeze date Aug 2nd to include it in the next Katib release ? |
As discussed with @kimwnasptd and @thesuperzapper in kubeflow#7085 Enabling RBAC Based on User Groups we aim for another approach than making the change of adding the support for another header. The goal would probably to migrate out of oidc-authservice in favor of oauth2-proxy, making a JWT token propagate instead of a plain text header. Closing this as will not be completed in this form. |
Description
The oidc-authservice support the
GROUPS_HEADER. In the current implementation theGroupsfield in theSubjectAccessReviewSpecis not used. Kubernetes have a build-in support for them.The format for the value of
GROUPS_HEADERis a string containing the groups separated with commas.By default
Kubeflowand specifically theauthn-filterenvoyFilter will not propagate thekubeflow-groupsheader only thekubeflow-userid, but this is only a configuration files, the changes in this PR will allow to get the groups the user is associated with, and create a subject review with its user-id (email) and groups.