Reducing the snapshot.storage.k8s.io group privileges

[OdedViner]

Procedure:
1. Deploy OCP4.17 [4.17.0-0.nightly-2024-08-31-032707]

2. Deploy ODF4.17 [odf-operator.v4.17.0-53.stable]

3.Verify storagecluster in Ready State
$ oc get storagecluster
NAME                 AGE   PHASE   EXTERNAL   CREATED AT             VERSION
ocs-storagecluster   20h   Ready              2024-09-01T11:48:56Z   4.17.0

4.Check clusterrole status:
// +kubebuilder:rbac:groups=snapshot.storage.k8s.io,resources=volumesnapshots;volumesnapshotclasses,verbs=*

5. Add "create" and "delete" verbs
delete: https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/volumesnapshotterclasses.go#L174
create: https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/volumesnapshotterclasses.go#L108
$ oc edit clusterrole ocs-operator.v4.17.0-53.-1QHgeXms7btcF6mudj63wa1ngH5P094kD8jAtR
- apiGroups:
  - snapshot.storage.k8s.io
  resources:
  - volumesnapshotclasses
  verbs:
  - create
  - delete
  - get
  - list
  - watch
- apiGroups:
  - snapshot.storage.k8s.io
  resources:
  - volumesnapshotclasses
  - volumesnapshots
  verbs:
  - '*'

- apiGroups:
  - snapshot.storage.k8s.io
  resources:
  - volumesnapshotclasses
  verbs:
  - create
  - delete
  - get
  - list
  - watch
- apiGroups:
  - snapshot.storage.k8s.io
  resources:
  - volumesnapshotclasses
  - volumesnapshots
  verbs:
  - create
  - delete


6.Delete ocs-operator pod and check ocs-operator pod logs:
$ oc delete pod ocs-operator

$ oc logs ocs-operator
[No errors]

7.Delete ocs-operator pod and check ocs-operator pod logs:
$ oc delete pod ocs-operator

8. Ruuning OCS-CI test:
tests/functional/pv/pvc_snapshot/test_restore_snapshot_using_different_sc.py [pass]

9.Add get [based code]
https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/volumesnapshotterclasses.go#L103

10. Change code:
// +kubebuilder:rbac:groups=snapshot.storage.k8s.io,resources=volumesnapshots;volumesnapshotclasses,verbs=get;create;delete

11.make gen-latest-csv:
export REGISTRY_NAMESPACE=ocs-dev
export IMAGE_TAG=latest
make gen-latest-csv

[iamniting]

We do need a watch as well

https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/storagecluster_controller.go#L232

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: OdedViner
Once this PR has been reviewed and has the lgtm label, please ask for approval from iamniting. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[iamniting]

We do need a watch as well

https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/storagecluster_controller.go#L232

@Nikhil-Ladha Can you pls help @OdedViner figure out if we need this permission or not, Unfortunately, the Pr is merged, If required we can get it added into the new PR.

[Nikhil-Ladha]

We do need a watch as well
https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/storagecluster_controller.go#L232

@Nikhil-Ladha Can you pls help @OdedViner figure out if we need this permission or not, Unfortunately, the Pr is merged, If required we can get it added into the new PR.

I think we need this verb for VolumeSnapshotClass.
IIUC, we don't create a VolumeSnapshotClass by default, so we never came across any error.
But, still interesting to see that no error is reported during the reconcile, if appropriate permission is not there 🤔

[OdedViner]

We do need a watch as well
https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/storagecluster_controller.go#L232

@Nikhil-Ladha Can you pls help @OdedViner figure out if we need this permission or not, Unfortunately, the Pr is merged, If required we can get it added into the new PR.

I think we need this verb for VolumeSnapshotClass. IIUC, we don't create a VolumeSnapshotClass by default, so we never came across any error. But, still interesting to see that no error is reported during the reconcile, if appropriate permission is not there 🤔

Because we have 2 roles for volumesnapshotclasses resource:

oc get clusterrole ocs-operator.v4.17
- apiGroups:
  - snapshot.storage.k8s.io
  resources:
  - volumesnapshotclasses
  verbs:
  - create
  - delete
  - get
  - list
  - watch
- apiGroups:
  - snapshot.storage.k8s.io
  resources:
  - volumesnapshotclasses
  - volumesnapshots
  verbs:
  - '*'

https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/reconcile.go#L117

https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagerequest/storagerequest_controller.go#L80

So I tested only volumesnapshots resource and not volumesnapshotclasses resource.
volumesnapshotclasses got the watch permission from here https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagerequest/storagerequest_controller.go#L80
@Nikhil-Ladha @iamniting

[iamniting]

We do need a watch as well
https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/storagecluster_controller.go#L232

@Nikhil-Ladha Can you pls help @OdedViner figure out if we need this permission or not, Unfortunately, the Pr is merged, If required we can get it added into the new PR.

I think we need this verb for VolumeSnapshotClass. IIUC, we don't create a VolumeSnapshotClass by default, so we never came across any error. But, still interesting to see that no error is reported during the reconcile, if appropriate permission is not there 🤔

Because we have 2 roles for volumesnapshotclasses resource:

oc get clusterrole ocs-operator.v4.17
- apiGroups:
  - snapshot.storage.k8s.io
  resources:
  - volumesnapshotclasses
  verbs:
  - create
  - delete
  - get
  - list
  - watch
- apiGroups:
  - snapshot.storage.k8s.io
  resources:
  - volumesnapshotclasses
  - volumesnapshots
  verbs:
  - '*'

https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/reconcile.go#L117

https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagerequest/storagerequest_controller.go#L80

So I tested only volumesnapshots resource and not volumesnapshotclasses resource. volumesnapshotclasses got the watch permission from here https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagerequest/storagerequest_controller.go#L80 @Nikhil-Ladha @iamniting

I believe we should have permission in both of the places. If for some reason we may alter the code or delete the code. There will be a regression. Can you pls raise the PR which adds watch permission in the storagecluster controller as well?

[OdedViner]

https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/storagecluster_controller.go#L232
Done
#2782 @Nikhil-Ladha @iamniting