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

[Bug]: 403 error creating model server with restricted permissions #1776

Closed
1 task done
christianvogt opened this issue Sep 11, 2023 · 23 comments
Closed
1 task done
Labels
feature/model-serving Model Serving Feature kind/bug Something isn't working migrated needs-info Further information is requested from the reporter or from another source priority/normal An issue with the product; fix when possible

Comments

@christianvogt
Copy link
Contributor

christianvogt commented Sep 11, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Deploy type

Downstream version (eg. RHODS 1.29)

Version

1.32

Current Behavior

Dev sandbox is a restricted multi-tenant environment where users only have access to a single namespace.

When trying to create a model server, an error is presented to the user but the model server seems to be created successfully.

The error occurs as ODH attempts to update the namespace resource for which it does not have access.
image

image

{
    "statusCode": 403,
    "code": "403",
    "error": "Forbidden",
    "message": "You don't have the access to update the namespace"
}

Expected Behavior

No error with successful model server creation.
OR
If the update to the namespace is crucial, then the model server probably shouldn't be created in the first place.
OR
Better messaging because it's not clear what is going on to the user even though the model server was created.

Steps To Reproduce

  1. Log in to dev sandbox
  2. Use the application launcher in the top right corner of the page and select Red Hat OpenShift Data Science
  3. Navigate to Data Science Projects
  4. Select the one pre-existing namespace
  5. Scroll down and click Add Server
  6. Complete the form.
  7. Click Add.
    Observe the 403 error but also that the model server is successfully created as seen when the modal is closed.

Workaround (if any)

No response

What browsers are you seeing the problem on?

No response

Anything else

No response

@christianvogt christianvogt added kind/bug Something isn't working untriaged Indicates the newly create issue has not been triaged yet priority/normal An issue with the product; fix when possible labels Sep 11, 2023
@github-project-automation github-project-automation bot moved this to Needs prioritization in ODH Dashboard Planning Sep 11, 2023
@andrewballantyne
Copy link
Member

@christianvogt you tested this on a cluster / ie not running it locally? The Service Account should manage this 🤔 -- is this reproducible on ODH?

@christianvogt
Copy link
Contributor Author

@andrewballantyne this was found on Red Hat Developer Sandbox

@lucferbux
Copy link
Contributor

oh wow, we fixed the rolebinding creation, but this seems to be targeted to the logic that adds an annotation in the project, this might totally be reproducible in ODH I think.

@lucferbux lucferbux added feature/model-serving Model Serving Feature and removed untriaged Indicates the newly create issue has not been triaged yet labels Sep 13, 2023
@lucferbux
Copy link
Contributor

It seems that our permissions are right, we might wanna check wether the dev sandbox has modified those permissions.

@lucferbux lucferbux moved this from Needs prioritization to To do in ODH Dashboard Planning Sep 13, 2023
@lucferbux lucferbux added the needs-info Further information is requested from the reporter or from another source label Sep 13, 2023
@christianvogt
Copy link
Contributor Author

This occurs because the SelfSubjectAccessReview check here.
Only users with permission to update the project can perform this operation. However in devsandbox users aren't admins of their projects.

An investigation into what is the correct permission to apply here needs to be done. At first thought it's likely that we should check if the user has view access to the project before granting access to update the namespace labels.

@bdattoma
Copy link

However in devsandbox users aren't admins of their projects.

@christianvogt @andrewballantyne @lucferbux do you think it is a real use case? If so, we should add this in our testing.

@andrewballantyne
Copy link
Member

@bdattoma It is a real use-case

  • Admin creates you a project
  • Grants you Edit access to the project
  • Doesn't add the modelmesh label to your namespace

This will error out. Our fix it to release some permission pressure. Essentially we are looking for "do you have the ability to write to the Project resource" and in reality we should be looking to see if you can create something IN the project... or view the project. We just are stopping the case of "I'm going to make the service account add the modelmesh label to someone else's project"... which is good for sanity, but we took it too far (first creation of Model Server) for non Admins of the project.

Once the label is added, no problem exists.

@bdattoma
Copy link

bdattoma commented Oct 2, 2023

  • Doesn't add the modelmesh label to your namespace

@andrewballantyne if the admin creates the project from RHODS Dashboard, the MM label should be automatically added, shouldn't it?
Is creating DS Project out of RHODS UI a supported user flow?

@andrewballantyne
Copy link
Member

andrewballantyne commented Oct 2, 2023

Is creating DS Project out of RHODS UI a supported user flow?

@bdattoma Yes gitops is a type of flow we have to start supporting better. Also creating the Project itself is done when you cannot allow your users to self provision new projects. These two points are real world use-cases we need to support in the Dashboard.

We are also removing it on initial creation. Adding it back on adding of the server. Two reasons for this:

  1. There is overhead to Model Mesh if we add it out of the gate -- [Bug]: Delay Model Serving Annotation until First Server #1310 (merged into a feature branch; needs to get into ODH atm)
  2. We want to support KServe -- [Tracker]: Model Serving v2 - KServe Support #1810

@bdattoma
Copy link

bdattoma commented Oct 3, 2023

Is creating DS Project out of RHODS UI a supported user flow?

@bdattoma Yes gitops is a type of flow we have to start supporting better. Also creating the Project itself is done when you cannot allow your users to self provision new projects. These two points are real world use-cases we need to support in the Dashboard.

We are also removing it on initial creation. Adding it back on adding of the server. Two reasons for this:

  1. There is overhead to Model Mesh if we add it out of the gate -- [Bug]: Delay Model Serving Annotation until First Server #1310 (merged into a feature branch; needs to get into ODH atm)
  2. We want to support KServe -- [Tracker]: Model Serving v2 - KServe Support #1810

Understood. Wouldn't it be better to have a clear set of requirements for the GitOps flow? So instead of adding some improvements here and there when it happens, we could have a clear view of what we need to implement and what is already in place.

@andrewballantyne
Copy link
Member

@bdattoma gitops flows are one of many reasons why we need to support a lazy add of the modelmesh. Should we have clear requirements for what needs to happen? Yes, but no one is writing them and it's just been sorta adhoc to what we do about it as it's reported. I'll inquire from Jeff to see if he has this on his roadmap.

@bdattoma
Copy link

bdattoma commented Oct 4, 2023

@bdattoma gitops flows are one of many reasons why we need to support a lazy add of the modelmesh. Should we have clear requirements for what needs to happen? Yes, but no one is writing them and it's just been sorta adhoc to what we do about it as it's reported. I'll inquire from Jeff to see if he has this on his roadmap.

Thank you! Keep us updated with that discussion :)

@bredamc
Copy link
Contributor

bredamc commented Oct 19, 2023

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

@lucferbux
Copy link
Contributor

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

This just happens in our dev sandbox, I guess it can be included but it's not something our customers will face in their clusters. @bredamc

@andrewballantyne
Copy link
Member

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

This just happens in our dev sandbox, I guess it can be included but it's not something our customers will face in their clusters. @bredamc

As I noted in the release notes doc... but I'll reply here as well for posterity.

We need to be careful about limitations being swept away as "only on x deployment"... if it is a problem on any cluster, it is a problem for the product. The priority is the only thing that changes. This issue is not cluster specific, but is the flow to creating DS Projects specifically. Admins creating the project for the user & not granting them admin but edit over it, is that that far fetched. #1776 (comment)

@bredamc
Copy link
Contributor

bredamc commented Oct 20, 2023

In that case, I think we should document it -- can one of you please provide the text for the release notes?

@bredamc
Copy link
Contributor

bredamc commented Oct 24, 2023

@andrewballantyne @bdattoma This issue is already documented in the 1.33 Release Notes with the following text:
"When you create a model server in an environment where you only have access to a single namespace, an Error creating model server error message appears. However, the model server is still successfully created."

Is that text correct, or should we replace it with the following text (or something similar):
"If you do not have administrator permissions on a project that you own, you cannot access some features, and the error messages do not explain why."

@bdattoma
Copy link

@andrewballantyne @bdattoma This issue is already documented in the 1.33 Release Notes with the following text: "When you create a model server in an environment where you only have access to a single namespace, an Error creating model server error message appears. However, the model server is still successfully created."

Is that text correct, or should we replace it with the following text (or something similar): "If you do not have administrator permissions on a project that you own, you cannot access some features, and the error messages do not explain why."

We could try merging the 2, sth like:

If you do not have administrator permissions on a project that you own, you cannot access some features, and the error messages do not explain why. For example, when you create a model server in an environment where you only have access to a single namespace, an Error creating model server error message appears. However, the model server is still successfully created

@andrewballantyne
Copy link
Member

I agree with Berto's text... I think the example helps out the explanation.

@bredamc
Copy link
Contributor

bredamc commented Oct 25, 2023

Do we want to change the section title, if the issue covers a wider range of problems?

"
ODH-DASHBOARD-1776 - Error appears when you create a model server in a single-namespace environment

If you do not have administrator permissions for a project that you own, you cannot access some features, and the error messages do not explain why. For example, when you create a model server in an environment where you only have access to a single namespace, an Error creating model server error message appears. However, the model server is still successfully created.
"

Maybe "Error messages when project owner does not have project administrator privileges" or something similar?

@andrewballantyne
Copy link
Member

@bredamc I think if we want to split hairs down to fine grain... we should say "Error messages when user does not have project administrator privileges". There is technically no special "ownership" to projects... just who is admin inside it.

@bredamc
Copy link
Contributor

bredamc commented Oct 25, 2023

Thank you, @andrewballantyne -- I am a big fan of fine grain :)

@dgutride
Copy link
Contributor

dgutride commented Dec 8, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/model-serving Model Serving Feature kind/bug Something isn't working migrated needs-info Further information is requested from the reporter or from another source priority/normal An issue with the product; fix when possible
Projects
Status: Done
Status: No status
Archived in project
Development

No branches or pull requests

6 participants