fix(charts): Remove unnecessary sensitive permissions for DaemonSet agent and Pod init

[kaaass]

What type of PR is this?

• release/bug

What this PR does / why we need it:

This PR removes unnecessary RBAC permissions for the DaemonSet agent and Pod init to avoid potential security risks. Technically, this PR does:

1. Create a separate ClusterRole for each workload
2. Remove some unnecessary permissions applied by the DaemonSet agent and Pod init

This PR minimizes the deletion of permissions to avoid affecting the normal function of the APP. However, there should still be some unnecessary RBAC permissions (such as for the Deployment controller).

Also, this PR DOES NOT include changing the kubebuilder annotation in: pkg/k8s/apis/spiderpool.spidernet.io/v2beta1/rbac.go

Which issue(s) this PR fixes:

Fixes #3420
Fixes #3361

Special notes for your reviewer:

The removed permission from the permission rules are as follows:

• DaemonSet agent

  • delete/deletecollection/patch/update verb of nodes/pods
    • As suggested in [BUG] Pod spiderpool-init has too much RBAC permission which may leads the whole cluster being hijacked #3420
  • update verb of daemonsets/deployments/replicasets/statefulsets
    • As suggested in [BUG] Pod spiderpool-init has too much RBAC permission which may leads the whole cluster being hijacked #3420
  • update verb of cronjobs/jobs
    • As suggested in [BUG] Pod spiderpool-init has too much RBAC permission which may leads the whole cluster being hijacked #3420
  • get/list/watch verb of *
    • As suggested in A potential risk in spiderpool that could lead to takeover of the cluster #3361, in order to remove get/list verb of secrets
    • This change needs to be carefully review since it contains a wildcard.
    • This change only deletes this rule, the get/list/watch permissions applied in other rules will not be affected.

• Pod init

  • delete/deletecollection/patch/update verb of nodes
  • update verb of daemonsets/deployments/replicasets/statefulsets
  • update verb of cronjobs/jobs

I am more aggressive in deleting the permissions of DaemonSet agent, mainly because DaemonSet will be deployed to any worker node in the cluster, so attackers have more opportunities to obtain the ServiceAccount token of DaemonSet agent.

[cyclinder]

Thanks for the work! @kaaass. Is it even better if we put resources with the same verbs into a list of the same resources? example:

- apiGroups:
  - spiderpool.spidernet.io
  resources:
  - spiderclaimparameters
  - spidercoordinators
  - spiderendpoints
  - spiderippools
  - spidermultusconfigs
  - spidersubnets
 verbs:
  - ...
  - ...
- apiGroups:
   - spiderpool.spidernet.io
   resources:
   - spidercoordinators/status
   - spiderippools/status
   - ...
   - ...

[kaaass]

Sure! I've updated them.

[kaaass]

Would it be possible to remove this rule (get/list/watch verb of *)? This rule include get/list verb of secrets at cluster-wide, which may cause some security risks. @cyclinder

[kaaass]

Also same question exists in Spiderpool controller.

[cyclinder]

Yes, You can boldly delete them. CI will tell us if we cannot get/list/watch some resources.

[kaaass]

OK! Let me give it a try

[ty-dc]

After the fix, some issues occurred, it seems that there is insufficient permissions.

  Warning  FailedCreatePodSandBox  118s                kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "91390322e1fcc5b6f8011c1f9292f3eaaba9db0b6704d79a8315413f16f05b04": plugin type="multus" name="multus-cni-network" failed (add): [default/test-pod-57c88b845c-prtfb/35ed07d0-ee08-4a64-963b-75dfeff0eb2c:macvlan-vlan0]: error adding container to network "macvlan-vlan0": failed to GetCoordinatorConfig: [GET /coordinator/config][500] getCoordinatorConfigFailure  spidercoordinator: default no ready

[kaaass]

Thanks @ty-dc. Seems like the removal in my latest commit is too aggressive. I'll revert them.

[cyclinder]

Hi @kaaass, codes look good now, Could you squash the comments? now we have 6 comments on the PR. it would be nice if we can squash to one comment.

[kaaass]

@cyclinder Sure! I've pushed them

[cyclinder]

Thanks for the work! @kaaass

[weizhoublue]

actually, I expect upgrading CI to test this PR @ty-dc

[ty-dc]

actually, I expect upgrading CI to test this PR @ty-dc

ok