Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Operator V2] Access to DS Projects fails if DS Pipelines are disabled from DSC #1870

Closed
bdattoma opened this issue Sep 7, 2023 · 25 comments · Fixed by #2018
Closed

[Operator V2] Access to DS Projects fails if DS Pipelines are disabled from DSC #1870

bdattoma opened this issue Sep 7, 2023 · 25 comments · Fixed by #2018
Assignees
Labels
feature/ds-projects Data Science Projects feature (formerly Data Science Groupings - DSG) feature/operator-rework Support for handling changes in the new operator rework effort. kind/bug Something isn't working priority/high Important issue that needs to be resolved asap. Releases should not have too many of these. rhods-1.35 rhods-2.4

Comments

@bdattoma
Copy link

bdattoma commented Sep 7, 2023

Describe the bug
When trying to access a Data Science Project using a non-cluster-admin user, we get a permission error related to pipelines-definition, which prevents us to use DS Projects UI

To Reproduce
Steps to reproduce the behavior:

  1. Install Operator V2
  2. Enable Dashboard and disable DS Pipelines in the DataScienceCluster object
  3. Log In Dashboard UI with a user who has no cluster admin permissions
  4. create a DS Project from Dashboard UI
  5. Access the DS Project from Dashboard UI
  6. See error

Expected behavior
No errors, user can use the DS Project UI page

Screenshots
image

Additional context

  • OpenShift Pipelines 1.11.1 operator is installed
  • DSC has dashboard,kserve and workbenches enabled only
  • enabling datasciencepipelines solves the issue
@zdtsw
Copy link
Member

zdtsw commented Sep 11, 2023

is this the expected behavior in v1 operator?
from the error message and the code, we do not grant permission in v2 explicitly in this case, but we need confirm if this should be the case, because in v1 we might open the permission a bit too wild.

@bdattoma
Copy link
Author

bdattoma commented Sep 12, 2023

is this the expected behavior in v1 operator?

This is not a case for operator v1 because we cannot disable components by using DSC

DS projects have more features than just Pipelines, so I believe a user of V2 operator can choose to enable workbenches and not pipelines. if that the case, the DS project must be accessible

@zdtsw
Copy link
Member

zdtsw commented Sep 13, 2023

so the real solution is we need to get the rules on datasciencepipelinesapplications with

    rbac.authorization.k8s.io/aggregate-to-admin: "true"
    rbac.authorization.k8s.io/aggregate-to-edit: "true" 

so the non-admin users can get admin privileges that will have the permissions specified in this ClusterRole.

purely with PR is not enough opendatahub-io/opendatahub-operator#529

@zdtsw
Copy link
Member

zdtsw commented Sep 13, 2023

and seems like we have several components actually are setting aggregate-to-admin for different clusterroles.

@zdtsw
Copy link
Member

zdtsw commented Sep 13, 2023

@harshad16 @atheo89 workbench and @lucferbux @andrewballantyne UI teams, could you help me to identify the issue here: if it is something from the workbench or dashboard or even operator?

TL;DR:
in v2 build, non-admin user get error message when access any Data Science Projects if the DSPA component is Removed.

offline discussed with Berto, this is a wrong behavior, that ODH operator does not need to have DSPA component Managed in order for non-admin user to view Data Science Projects.

in v1, since all components are by default enabled => ClusterRole with aggragated admin permission are added, but in v2, we only create such if the component is Managed.

So the question is, why it requires such permission if non-admin user wants to only access Data Science Projects.

@zdtsw zdtsw removed the rhods-1.33 label Sep 13, 2023
@bdattoma
Copy link
Author

the workaround seems to be disabling pipelines from OdhDashboardConfig instance: disablePipelines: false

@harshad16
Copy link
Member

This seems like UI people would be able to answer better than the notebooks-teams.
some follow up question , that might help with the discussion:

  • Does removing the workbench component , cause the same issue ?
  • Does removing the model serving component, cause the same issue?

probably as current in the data science project instance: workbenches, data science pipeline and model serving
are provided, removing them breaks the flow of the data science project.

@bdattoma
Copy link
Author

@harshad16 I tried removing model serving component, and the behavior is different: user is able to access their DS Project, but an error msg is rendered in the model serving section (see the screenshot below).

image

@harshad16
Copy link
Member

Seems like then the close relation is only with data science pipeline, perhaps as you mentioned the workaround
that is the content causing this close bond.
UI team would be the best to answer this.

@andrewballantyne
Copy link
Member

This definitely seems like a Dashboard issue. Interesting state of K8s to give a 403 error if the CRD does not exist for basic users.

/transfer odh-dashboard

Hopefully the bot exists in this repo 🙂

@bdattoma
Copy link
Author

thank you all for looking into it :)

@zdtsw
Copy link
Member

zdtsw commented Sep 27, 2023

/transfer odh-dashboard

@openshift-ci openshift-ci bot transferred this issue from opendatahub-io/opendatahub-operator Sep 27, 2023
@Gkrumbach07 Gkrumbach07 added kind/bug Something isn't working feature/ds-projects Data Science Projects feature (formerly Data Science Groupings - DSG) priority/high Important issue that needs to be resolved asap. Releases should not have too many of these. labels Oct 2, 2023
@andrewballantyne
Copy link
Member

I imagine this has a reliance on opendatahub-io/opendatahub-operator#588 in some way. We could probably handle the error for this ticket by just not failing like this... but to get it right we need to rework our feature flags & rely on the DSC information.

@bdattoma
Copy link
Author

bdattoma commented Oct 4, 2023

but to get it right we need to rework our feature flags & rely on the DSC information.

makes sense to me. However, is there still a value in having the additional feature flags in the OdhDashboardConfig CR? If yes, then the dashboard should rely on the combination of both

@andrewballantyne
Copy link
Member

andrewballantyne commented Oct 4, 2023

@bdattoma that's being debated... but yes, that's our current plan.

@andrewballantyne andrewballantyne self-assigned this Oct 16, 2023
@andrewballantyne andrewballantyne added the feature/operator-rework Support for handling changes in the new operator rework effort. label Oct 18, 2023
@bredamc
Copy link
Contributor

bredamc commented Oct 19, 2023

@harshad16 If you would like to include this issue in the 1.34 Release Notes, please provide the text for the "Known issues" section.

@andrewballantyne
Copy link
Member

@harshad16 If you would like to include this issue in the 1.34 Release Notes, please provide the text for the "Known issues" section.

@bredamc This is a known issue for v2 operator, I don't think it applies to the v1 operator for 1.34. Aimed to be fixed in 1.35

@bredamc
Copy link
Contributor

bredamc commented Oct 19, 2023

Thanks @andrewballantyne. We already mention the V2 Operator in the "Limited Availability features" section of the 1.33 Release Notes, so I wondered if we should include this issue. Fine with me to exclude it in 1.34 :)

@andrewballantyne
Copy link
Member

Thanks @andrewballantyne. We already mention the V2 Operator in the "Limited Availability features" section of the 1.33 Release Notes, so I wondered if we should include this issue. Fine with me to exclude it in 1.34 :)

@bredamc I don't fully know the rules of LA, so I'll follow your lead on what we want to report.

@bredamc
Copy link
Contributor

bredamc commented Oct 24, 2023

@andrewballantyne We can include this issue in the self-managed version of the 1.34 Release Notes.

Here is some suggested text, please advise if we need to change it. If a workaround is available, please specify.

"
ODH-DASHBOARD-1870 - Access to projects fails if pipelines are disabled from cluster

Red Hat OpenShift Data Science Operator V2 only: If pipelines are disabled in the DataScienceCluster object, users who are not cluster admins cannot access projects that they created. Red Hat OpenShift Data Science Operator V2 is a Limited Availability feature.
"

@andrewballantyne
Copy link
Member

@bredamc so we need to address this two ways:

  1. The workaround here is to disable DS Pipelines feature flag OdhDashboardConfig => spec.dashboardConfig.disablePipelines = true
  2. I don't believe this has anything to do with permissions, I think this is just the lack of the backend component being available

So something like:

ODH-DASHBOARD-1870 - Access to projects fails if pipelines are disabled from cluster

Red Hat OpenShift Data Science Operator V2 only: If pipelines are disabled in the DataScienceCluster object, users cannot access projects that they created. Red Hat OpenShift Data Science Operator V2 is a Limited Availability feature.

Workaround: Disable the pipelines UI in the OdhDashboardConfig (.spec.dashboardConfig.disablePipelines = true)

@bredamc
Copy link
Contributor

bredamc commented Oct 25, 2023

@andrewballantyne Thank you for the workaround -- is the following text correct?

"
Complete the following steps as a user with cluster-admin permissions:

  1. Log in to your cluster using the oc client.
  2. Enter the following command to update the OdhDashboardConfig Custom Resource Definition (CRD) in the redhat-ods-applications application namespace:

$ oc patch OdhDashboardConfig odh-dashboard-config -n redhat-ods-applications --type=merge -p '{"spec": {"dashboardConfig": {"disablePipelines": true}}}'

"

@andrewballantyne
Copy link
Member

@bredamc Small correction... it's a "CR" they are modifying, not a "CRD". The CRDs are definitions, "blueprints"; in this case it already supports the ability to disable. The CRs are resources, these are data.

The patch command should work.

@bredamc
Copy link
Contributor

bredamc commented Oct 25, 2023

Great -- thanks, Andrew!

@manosnoam
Copy link

Verified scenario on RHODS 2.4:

image

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/ds-projects Data Science Projects feature (formerly Data Science Groupings - DSG) feature/operator-rework Support for handling changes in the new operator rework effort. kind/bug Something isn't working priority/high Important issue that needs to be resolved asap. Releases should not have too many of these. rhods-1.35 rhods-2.4
Projects
Status: Done
Archived in project
Status: No status
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants