BUG: Unnecessary RBAC permissions about volcano HOT 10 CLOSED

1、volcano admission need generate secrets in a init job, and I will fire a pr to shrink permissions.
2、volcano controller's permissions of list/watch secrets have been removed in latest version.
3、update permission is necessary for volcano controller because it need update pod's podgroup, etc.

from volcano.

@Monokaix I tried to find all update/patch relate operation to pods. Maybe we only need the patch verb instead of update verb?

from volcano.

similar issues : A potential risk in volcano that could lead to takeover of the cluster #3446

from volcano.

@Monokaix I tried to find all update/patch relate operation to pods. Maybe we only need the patch verb instead of update verb?

volcano/pkg/scheduler/api/devices/nvidia/vgpu/utils.go

Lines 437 to 438 in fa0548f

	_, err = kubeClient.CoreV1().Pods(pod.Namespace).
	Patch(context.Background(), pod.Name, k8stypes.StrategicMergePatchType, bytes, metav1.PatchOptions{})

volcano/pkg/controllers/podgroup/pg_controller_handler.go

Line 105 in fa0548f

if _, err := pg.kubeClient.CoreV1().Pods(pod.Namespace).Patch(context.TODO(), pod.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}); err != nil {

volcano/pkg/scheduler/api/devices/nvidia/gpushare/device_info.go

Line 136 in fa0548f

_, err := kubeClient.CoreV1().Pods(pod.Namespace).Patch(context.TODO(), pod.Name, types.JSONPatchType, []byte(patch), metav1.PatchOptions{})

volcano/pkg/scheduler/api/devices/nvidia/gpushare/device_info.go

Line 195 in fa0548f

pod, err := kubeClient.CoreV1().Pods(pod.Namespace).Patch(context.TODO(), pod.Name, types.JSONPatchType, []byte(patch), metav1.PatchOptions{})

volcano/pkg/scheduler/api/devices/nvidia/gpushare/device_info.go

Line 212 in fa0548f

pod, err := kubeClient.CoreV1().Pods(pod.Namespace).Patch(context.TODO(), pod.Name, types.JSONPatchType, []byte(patch), metav1.PatchOptions{})

After a deep insight, volcano scheduler called UpdateStatus method, which needs update verb role: )

from volcano.

And volcano admission related permissions has been reduced in pr #3504

from volcano.

@kaaass I think we can remove update verb in volcano controller, you can do that if you're available: )

from volcano.

After a deep insight, volcano scheduler called UpdateStatus method, which needs update verb role: )

@Monokaix Thank you for reply : )

UpdateStatus only requires permission to subresource pods/status (client-go source code). Subresource uses a separate permission grants (document).

@kaaass I think we can remove update verb in volcano controller, you can do that if you're available: )

I'm happy to do that! Sadly I'm a little busy at the time. I'll give it a try if it is still unsolved maybe later this week : )

from volcano.

After a deep insight, volcano scheduler called UpdateStatus method, which needs update verb role: )

@Monokaix Thank you for reply : )

UpdateStatus only requires permission to subresource pods/status (client-go source code). Subresource uses a separate permission grants (document).

@kaaass I think we can remove update verb in volcano controller, you can do that if you're available: )

I'm happy to do that! Sadly I'm a little busy at the time. I'll give it a try if it is still unsolved maybe later this week : )

That's ok.

from volcano.

/close

from volcano.

@Monokaix: Closing this issue.

from volcano.