Istio 1.4.0 ClusterRole for Pilot is a bit confusing

[Stono]

Bug description
The ClusterRole for pilot is a little confusing, firstly secrets are defined twice:

- apiGroups:
  - ''
  resources:
  - endpoints
  - pods
  - services
  - namespaces
  - nodes
  - secrets
  verbs:
  - get
  - list
  - watch
- apiGroups:
  - ''
  resources:
  - secrets
  verbs:
  - create
  - get
  - watch
  - list
  - update
  - delete

The second block (added in 1.4.0) gives pilot permission to create, update and delete secrets too. What is that permission used for?

Expected behavior
The RBAC config to be concise and an explanation in the upgrade notes as to any privilege changes.

Steps to reproduce the bug

Version (include the output of istioctl version --remote and kubectl version and helm version if you used Helm)
1.4.0

How was Istio installed?
Helm

Environment where bug was observed (cloud vendor, OS, etc)

[howardjohn]

@lei-tang was the secret stuff for DNS certs? Is it always added?

[JimmyCYJ]

@lei-tang Could you please take a look?

[lei-tang]

The permission to create, update and delete secrets is for Pilot (through Chiron linked with Pilot) to create, update and delete DNS certificates (e.g., for Galley and Sidecar Injector) in secrets.

[Stono]

OK that's fine, so I have a few questions:

1. We don't use that feature, is it something that is turned on an off with a helm config? If so, should it also remove these permissions if not used?
2. When you change (increase) RBAC for a component between releases, it would be good to have in the changelog the reasoning behind the elevated privileges.
3. As mentioned in the issue above, secrets are in multiple rule groups right now, so can be removed from the lesser get, list, watch group.

[lei-tang]

The DNS certificates can be turned on/off with a helm config in https://github.com/istio/istio/blob/release-1.4/install/kubernetes/helm/istio/values.yaml#L605. When the DNS certificate configuration is empty, no DNS certificates are generated. If DNS certificates are not used, the permission to create, update and delete DNS certificates can be removed.

The PR #19332 is created to remove secret permission from the lesser get, list, watch group.

[Stono]

If DNS certificates are not used, the permission to create, update and delete DNS certificates can be removed.

Perhaps we should have this automatically happen in the template for the ClusterRole? Most operators aren't going to go and edit the generated ClusterRole.

[howardjohn]

Perhaps we should have this automatically happen in the template for the ClusterRole? Most operators aren't going to go and edit the generated ClusterRole.

👍 I agree. DNS certs can be turned on or off at install time, so we should update the cluster role as well

[lei-tang]

#19357 is created to automatically configure the ClusterRole based on certificates setting.