red-hat-storage / ocs-operator

Operator for RHOCS
Apache License 2.0
86 stars 184 forks source link

Reducing the apps group privileges #2716

Closed OdedViner closed 2 weeks ago

OdedViner commented 1 month ago
1.Deploy OCP4.17

2.Install ODF4.17  [4.17.0-29.stable]

3.Check clusterrole status:
// +kubebuilder:rbac:groups=apps,resources=deployments;daemonsets;replicasets;statefulsets,verbs=*

$ oc get clusterrole ocs-operator.v4.17.0-29.-a2YWGo4YLCr5DJ6FdgHYR9jOIUTJOWs7bLd3qO -o yaml | grep deployment -C10
- apiGroups:
  - apps
  resources:
  - daemonsets
  - deployments
  - replicasets
  - statefulsets
  verbs:
  - '*'

4. Add create verb ocs-operator clusterrole status:
Add [create]
$ oc edit clusterrole ocs-operator.v4.17.0-29.-a2YWGo4YLCr5DJ6FdgHYR9jOIUTJOWs7bLd3qO 
- apiGroups:
  - apps
  resources:
  - daemonsets
  - deployments
  - replicasets
  - statefulsets
  verbs:
  - create

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

$ oc logs ocs-operator-7d49979444-l58wr
list:
W0729 11:34:15.923096       1 reflector.go:539] sigs.k8s.io/controller-runtime/pkg/cache/internal/informers.go:105: failed to list *v1.Deployment: deployments.apps is forbidden: User "system:serviceaccount:openshift-storage:ocs-operator" cannot list resource "deployments" in API group "apps" in the namespace "openshift-storage"

watch:
E0729 11:34:15.923128       1 reflector.go:147] sigs.k8s.io/controller-runtime/pkg/cache/internal/informers.go:105: Failed to watch *v1.Deployment: failed to list *v1.Deployment: deployments.apps is forbidden: User "system:serviceaccount:openshift-storage:ocs-operator" cannot list resource "deployments" in API group "apps" in the namespace "openshift-storage"

6.Add "list" and "watch":
oc edit clusterrole ocs-operator.v4.17.0-29.-a2YWGo4YLCr5DJ6FdgHYR9jOIUTJOWs7bLd3qO 
- apiGroups:
  - apps
  resources:
  - daemonsets
  - deployments
  - replicasets
  - statefulsets
  verbs:
  - create
  - list
  - watch

7.Delete ocs-operator pod and check ocs-operator pod logs:
$ oc delete pod ocs-operator
$oc logs ocs-operator
{"level":"error","ts":"2024-07-29T11:41:39Z","msg":"Reconciler error","controller":"storagecluster","controllerGroup":"ocs.openshift.io","controllerKind":"StorageCluster","StorageCluster":{"name":"ocs-storagecluster","namespace":"openshift-storage"},"namespace":"openshift-storage","name":"ocs-storagecluster","reconcileID":"6258532a-f28a-421f-a9bd-72e0055f1f99","error":"deployments.apps \"rook-ceph-tools\" is forbidden: User \"system:serviceaccount:openshift-storage:ocs-operator\" cannot update resource \"deployments\" in API group \"apps\" in the namespace \"openshift-storage\"","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/remote-source/app/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:329\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/remote-source/app/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:266\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\t/remote-source/app/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:227"}

8.Add "update":
oc edit clusterrole ocs-operator.v4.17.0-29.-a2YWGo4YLCr5DJ6FdgHYR9jOIUTJOWs7bLd3qO 
- apiGroups:
  - apps
  resources:
  - daemonsets
  - deployments
  - replicasets
  - statefulsets
  verbs:
  - create
  - list
  - watch
  - update

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

{"level":"error","ts":"2024-07-29T11:44:24Z","msg":"Reconciler error","controller":"storagecluster","controllerGroup":"ocs.openshift.io","controllerKind":"StorageCluster","StorageCluster":{"name":"ocs-storagecluster","namespace":"openshift-storage"},"namespace":"openshift-storage","name":"ocs-storagecluster","reconcileID":"d80690cf-c6eb-45dc-a54a-10db9586aedf","error":"deployments.apps \"ocs-provider-server\" is forbidden: User \"system:serviceaccount:openshift-storage:ocs-operator\" cannot delete resource \"deployments\" in API group \"apps\" in the namespace \"openshift-storage\"","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/remote-source/app/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:329\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/remote-source/app/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:266\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\t/remote-source/app/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:227"}

10.Add "get" and "delete":
oc edit clusterrole ocs-operator.v4.17.0-29.-a2YWGo4YLCr5DJ6FdgHYR9jOIUTJOWs7bLd3qO 
- apiGroups:
  - apps
  resources:
  - daemonsets
  - deployments
  - replicasets
  - statefulsets
  verbs:
  - create
  - list
  - watch
  - update
  - get
  - delete

11.Delete ocs-operator pod and check ocs-operator pod logs:
$ oc delete pod ocs-operator
$ oc logs ocs-operator
[No Error]

12.Check OCS CRs stautus 
$ oc get storageclusters.ocs.openshift.io 
NAME                 AGE   PHASE   EXTERNAL   CREATED AT             VERSION
ocs-storagecluster   21h   Ready              2024-07-28T14:36:03Z   4.17.0

$ oc get cephclusters.ceph.rook.io 
NAME                             DATADIRHOSTPATH   MONCOUNT   AGE   PHASE   MESSAGE                        HEALTH      EXTERNAL   FSID
ocs-storagecluster-cephcluster   /var/lib/rook     3          21h   Ready   Cluster created successfully   HEALTH_OK              bfe74cbf-3fa9-4cbe-928a-2377853ffb5d

$ oc get ocsinitializations.ocs.openshift.io 
NAME      AGE   PHASE   CREATED AT
ocsinit   21h   Ready   2024-07-28T14:33:28Z
OdedViner commented 1 month ago

Few questions/changes:

  • Are all the permissions really needed? -> As you can see in the description of the PR, the testing process was serial and I checked the logs in ocs operator pod
  • Please add a meaningful commit message to the commit. -> can you send me example
  • Split the generated changes to a separate commit. -> got it
Nikhil-Ladha commented 1 month ago

Ex commit msg: https://github.com/red-hat-storage/ocs-operator/pull/2720/commits/338cf6910f8350fd8789baefadb09e8e286c2ce0

Nikhil-Ladha commented 3 weeks ago

/retest

Nikhil-Ladha commented 3 weeks ago

Please remove the bz number from the commit msg, let it be generic to the code.

OdedViner commented 3 weeks ago

Please remove the bz number from the commit msg, let it be generic to the code.

done

Nikhil-Ladha commented 3 weeks ago

/retest

openshift-ci[bot] commented 2 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: iamniting, Nikhil-Ladha, OdedViner

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/red-hat-storage/ocs-operator/blob/main/OWNERS)~~ [iamniting] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment