Open gaktive opened 1 year ago
I think, to summarise...
PATCH
permission on resourcePOST
permission on resource (uses update
link for visibility of feature)The screenshot is of ember using RKE1 to change node pool. Need to also consider RKE2 and machine deployment scaling
Confirmed that this was spotted in 2.7-head with the Vue UI.
We should change scale to use PATCH
to be consistent with kubectl.
Suggest we only do this for the Vue codebase for now.
@richard-cox indicated that we should use PATCH
instead of POST
for scaling +/-.
In addition, @Sean-McQ noted that the PATCH
verb is a partial update to a resource; we only need to send a diff on the fields that change. PUT
is the other preferred verb to use instead of POST
going forward (PUT
overrides the full resource).
This use PUT
. This mean when creating the project role user need to have Update
label instead of the patch.
@nwmac Do we need to update things to use PATCH
? we are not using patch any where in the codebase.
@Shavindra For scale up/down I think we do - this is then consistent with what kubectl
does.
I've created a backend issue. Schema/Links are not updated based on PATCH
permissions. This issue is blocked by this.
https://github.com/rancher/rancher/issues/40712
CC: @nwmac @gaktive
Internal reference: SURE-3183 Reported in 2.5.8
Issue description: A user has a read-only role with additional "scale" permissions so that app users can only scale their app and cannot make any other modifications.
With this role, it's not possible to scale up/down a deployment. But the same is possible via
kubectl
.Impact:
Although they can bring up the shell and scale manually, a primary differentiating point of rancher was the ability to interact with the k8s cluster via a GUI as not all of their devs are familiar with the kubectl CLI.
Repro steps:
kubectl scale deployments/test --replicas=2
. This goes through successfully.Expected behavior: Scaling via UI should work
Notes: Initial thoughts showed possible connection to https://github.com/rancher/rancher/issues/32424 but upon backend investigation elsewhere, @crobby spotted the following with recent testing (confirming version):