rancher / dashboard

The Rancher UI
https://rancher.com
Apache License 2.0
452 stars 258 forks source link

Need to rethink Create Namespace button in certain views #5120

Closed gaktive closed 2 years ago

gaktive commented 2 years ago

@gaktive I took a look into this on the UI side. Relevant code (that I think you were referring to) is here: https://github.com/rancher/dashboard/blob/db6bca553e54d7dac2b8cc593e79e176581dbfc5/pages/c/_cluster/_product/projectsnamespaces.vue#L50

I think I understand the original intention of the UI side of this code. That being said, I think there is a large flaw to this approach, namely that the button's presence is tied (in best case) to the user's ability to create namespaces generically. This leads to the button being present on every project that the user sees, or on no projects. I'm not sure that this is the desired behavior. It seems to me that it would be better to evaluate if the button should be present on a project-by-project basis. That being said, the way that you are determining permissions right now (through the schema) doesn't really seem to support this. I'm going to look more into the backend side and see if there's something that makes more sense there.

Originally posted by @MbolotSuse in https://github.com/rancher/rancher/issues/35453#issuecomment-1036667181

gaktive commented 2 years ago

Update from @MbolotSuse from https://github.com/rancher/rancher/issues/35453#issuecomment-1045177528 (pardon any formatting wackiness here):

When Steve checks for access permissions to create today, it looks for the "POST" ability contained in the "CollectionMethods" field of the schema. You can see that here: https://github.com/rancher/apiserver/blob/master/pkg/server/access.go#L17-L22 .

The UI, on the other hand, is currently checking if the user has "PUT" access to "ResourceMethods". This checks if the user has update access to a namespace, rather than create permissions to namespaces. Because the project in this issue is initially empty, the backend does not report that the user has "PUT" access to "ResourceMethods" (because there exists no namespaces that the user has access to modify). Once a namespace has been created, however, the user now has update/edit access to a namespace, which causes the button to appear on the UI.

The solution here is pretty simple.

https://github.com/rancher/dashboard/blob/db6bca553e54d7dac2b8cc593e79e176581dbfc5/pages/c/_cluster/_product/projectsnamespaces.vue#L51 should be changed from:

return (this.schema?.resourceMethods || []).includes('PUT');

to

return (this.schema?.collectionMethods || []).includes('POST');

I ran a quick test on my local using this as the override, and was able to get the button to show up as expected (doesn't show up when the user is read-only, does when the user is project member).

As far as my earlier comment goes, the backend logic doesn't support the structure that I was referring to there. That may be a topic that we revisit in the future, but for now I would recommend going wiht the above fix.

gaktive commented 2 years ago

@Sean-McQ to take a quick look to see if it as easy as a one-line fix or if this code touches a lot of other parts of the dashboard which may present UI with some design questions.

gaktive commented 2 years ago

@Sean-McQ can you look at @MbolotSuse's feedback for your PR here?

sowmyav27 commented 2 years ago

@Sean-McQ @nwmac Can you please add some info on what should be validated and what was fixed? Please fill the dev template with info for QA

nwmac commented 2 years ago

@Sean-McQ Can you respond on this one and then move back to 'to test'.

ghost commented 2 years ago

see this issue for repro details: https://github.com/rancher/rancher/issues/35453#issuecomment-1036667181

brudnak commented 2 years ago

:white_check_mark: PASSED

Reproduction Environment

Component Version / Type
Rancher version 2.6.3
Installation option Helm high availability
If Helm Chart k8s cluster RKE v1.22.6
Cert Details ingress.tls.source=secret (certificates from files)
Docker version 20.10.7, build f0df350
Helm version v2.16.8-rancher1
Downstream cluster type RKE1 Linode
Downstream K8s version v1.21.9-rancher1-1
Logged in user role Local standard user
Browser type Google Chrome
Browser version 99.0.4844.51 (Official Build) (x86_64)

Reproduction steps

  1. Create a downstream cluster
  2. Create a local standard user
  3. As the global admin create a project in the downstream cluster
  4. Add the standard user as a project owner
  5. Login as the standard user
  6. Try to create a namespace in the project you were assigned

Additional Info

RESULTS

:white_check_mark: Expected

As a project owner I would expect to be able to create a namespace

:x: Actual

As a project owner I am not able to create a namespace


Validation Environment

Component Version / Type
Rancher version 2.6.4-rc9
Installation option Helm high availability
If Helm Chart k8s cluster RKE v1.22.6
Cert Details ingress.tls.source=secret (certificates from files)
Docker version 20.10.7, build f0df350
Helm version v2.16.8-rancher1
Downstream cluster type RKE1 Linode
Downstream K8s version v1.23.4-rancher1-1
Logged in user role Local standard user
Browser type Google Chrome
Browser version 99.0.4844.51 (Official Build) (x86_64)

Validation steps

  1. Create a downstream cluster
  2. Create a local standard user
  3. As the global admin create a project in the downstream cluster
  4. Add the standard user as a project owner
  5. Login as the standard user
  6. Try to create a namespace in the project you were assigned

Additional Info

RESULTS

:white_check_mark: Expected

As a project owner I would expect to be able to create a namespace

:white_check_mark:Actual

As a project owner I am able to create namespaces within the project

nelsonroliveira commented 10 months ago

@gaktive I took a look into this on the UI side. Relevant code (that I think you were referring to) is here:

https://github.com/rancher/dashboard/blob/db6bca553e54d7dac2b8cc593e79e176581dbfc5/pages/c/_cluster/_product/projectsnamespaces.vue#L50

I think I understand the original intention of the UI side of this code. That being said, I think there is a large flaw to this approach, namely that the button's presence is tied (in best case) to the user's ability to create namespaces generically. This leads to the button being present on every project that the user sees, or on no projects. I'm not sure that this is the desired behavior. It seems to me that it would be better to evaluate if the button should be present on a project-by-project basis. That being said, the way that you are determining permissions right now (through the schema) doesn't really seem to support this. I'm going to look more into the backend side and see if there's something that makes more sense there.

Originally posted by @MbolotSuse in rancher/rancher#35453 (comment)

We appear to be seeing a variation of this issue that your comment seams to predict...

If a user has access to project A but not project B, the "create namespace" will show on both even though it will then fail to create the namespace on project B

gaktive commented 10 months ago

@nelsonroliveira can you file a new ticket and link back to this for context? This ticket is quite old (2.6.4 vs. 2.8.x) so opening something new allows for a fresh look at this.

If you can also include steps to reproduce what you're seeing in that new ticket, that'd help.