Generate RBAC settings declaratively

This allows RBAC settings to be declared as close as possible to their point of use, which means that, as functions are added and deleted, permissions will be adjusted "automatically" and we'll avoid keeping no-longer-needed permissions. As generated by the operator SDK, the operator ends up with only cluster roles, but this makes sense since the operator is supposed to be able to act in any namespace. Fixes: submariner-io#1105 Signed-off-by: Stephen Kitt <[email protected]>
skitt · Mar 19, 2021 · 941914b · 941914b
1 parent 6139040
commit 941914b
Show file tree

Hide file tree

Showing 18 changed files with 274 additions and 60 deletions.
diff --git a/.yamllint.yml b/.yamllint.yml
@@ -21,6 +21,7 @@ ignore: |
   /config/crd/bases/submariner.io_brokers.yaml
   /config/crd/bases/submariner.io_submariners.yaml
   /config/crd/bases/submariner.io_servicediscoveries.yaml
+  /config/rbac/submariner-operator/role.yaml
   /config/manager/kustomization.yaml
   /config/manifests/bases/submariner.clusterserviceversion.yaml
   /bundle

diff --git a/Makefile b/Makefile
@@ -150,7 +150,7 @@ generate: vendor/modules.txt
 
 # Generate manifests e.g. CRD, RBAC etc
 manifests: generate vendor/modules.txt
-	controller-gen $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
+	controller-gen $(CRD_OPTIONS) rbac:roleName=submariner-operator webhook paths="./..." output:crd:artifacts:config=config/crd/bases output:rbac:artifacts:config=config/rbac/submariner-operator
 
 # test if VERSION matches the semantic versioning rule
 is-semantic-version:

diff --git a/config/rbac/submariner-operator/role.yaml b/config/rbac/submariner-operator/role.yaml
@@ -1,63 +1,198 @@
+
 ---
 apiVersion: rbac.authorization.k8s.io/v1
-kind: Role
+kind: ClusterRole
 metadata:
   creationTimestamp: null
   name: submariner-operator
 rules:
-  - apiGroups:
-      - ""
-    resources:
-      - pods
-      - services
-      - services/finalizers
-      - endpoints
-      - persistentvolumeclaims
-      - events
-      - configmaps
-      - secrets
-    verbs:
-      - '*'
-  - apiGroups:
-      - apps
-    resources:
-      - deployments
-      - daemonsets
-      - replicasets
-      - statefulsets
-    verbs:
-      - '*'
-  - apiGroups:
-      - monitoring.coreos.com
-    resources:
-      - servicemonitors
-    verbs:
-      - get
-      - create
-  - apiGroups:
-      - apps
-    resourceNames:
-      - submariner-operator
-    resources:
-      - deployments/finalizers
-    verbs:
-      - update
-  - apiGroups:
-      - ""
-    resources:
-      - pods
-    verbs:
-      - get
-  - apiGroups:
-      - apps
-    resources:
-      - replicasets
-    verbs:
-      - get
-  - apiGroups:
-      - submariner.io
-    resources:
-      - '*'
-      - servicediscoveries
-    verbs:
-      - '*'
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  verbs:
+  - create
+  - get
+  - update
+- apiGroups:
+  - ""
+  resources:
+  - namespaces
+  verbs:
+  - create
+- apiGroups:
+  - ""
+  resources:
+  - nodes
+  verbs:
+  - list
+  - patch
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - list
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+- apiGroups:
+  - ""
+  resources:
+  - serviceaccounts
+  verbs:
+  - create
+  - get
+  - update
+- apiGroups:
+  - ""
+  resources:
+  - services
+  verbs:
+  - create
+  - get
+  - update
+- apiGroups:
+  - apiextensions.k8s.io
+  resources:
+  - customresourcedefinitions
+  verbs:
+  - create
+  - get
+  - update
+- apiGroups:
+  - apps
+  resources:
+  - daemonsets
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - update
+  - watch
+- apiGroups:
+  - apps
+  resources:
+  - deployments
+  verbs:
+  - create
+  - get
+  - list
+  - update
+  - watch
+- apiGroups:
+  - apps
+  resources:
+  - replicasets
+  verbs:
+  - get
+- apiGroups:
+  - rbac
+  resources:
+  - clusterrolebindings
+  verbs:
+  - create
+  - get
+  - update
+- apiGroups:
+  - rbac
+  resources:
+  - clusterroles
+  verbs:
+  - create
+  - get
+  - update
+- apiGroups:
+  - rbac
+  resources:
+  - rolebindings
+  verbs:
+  - create
+  - get
+  - update
+- apiGroups:
+  - rbac
+  resources:
+  - roles
+  verbs:
+  - create
+  - get
+  - update
+- apiGroups:
+  - submariner.io
+  resources:
+  - brokers
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - submariner.io
+  resources:
+  - brokers/status
+  verbs:
+  - get
+  - patch
+  - update
+- apiGroups:
+  - submariner.io
+  resources:
+  - gateways
+  verbs:
+  - list
+- apiGroups:
+  - submariner.io
+  resources:
+  - gateways
+  - submariners
+  verbs:
+  - list
+  - watch
+- apiGroups:
+  - submariner.io
+  resources:
+  - servicediscoveries
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - submariner.io
+  resources:
+  - servicediscoveries/status
+  verbs:
+  - get
+  - patch
+  - update
+- apiGroups:
+  - submariner.io
+  resources:
+  - submariners
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - submariner.io
+  resources:
+  - submariners/status
+  verbs:
+  - get
+  - patch
+  - update
diff --git a/controllers/helpers/helpers.go b/controllers/helpers/helpers.go
@@ -34,6 +34,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
+// +kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=create;delete;get;update
+
 func ReconcileDaemonSet(owner metav1.Object, daemonSet *appsv1.DaemonSet, reqLogger logr.Logger,
 	client controllerClient.Client, scheme *runtime.Scheme) (*appsv1.DaemonSet, error) {
 	var err error
@@ -94,6 +96,8 @@ func ReconcileDaemonSet(owner metav1.Object, daemonSet *appsv1.DaemonSet, reqLog
 	return daemonSet, errorutil.WithMessagef(err, "error creating or updating DaemonSet %s/%s", daemonSet.Namespace, daemonSet.Name)
 }
 
+// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;get;update
+
 func ReconcileDeployment(owner metav1.Object, deployment *appsv1.Deployment, reqLogger logr.Logger,
 	client controllerClient.Client, scheme *runtime.Scheme) (*appsv1.Deployment, error) {
 	var err error
@@ -140,6 +144,8 @@ func ReconcileDeployment(owner metav1.Object, deployment *appsv1.Deployment, req
 	return deployment, errorutil.WithMessagef(err, "error creating or updating Deployment %s/%s", deployment.Namespace, deployment.Name)
 }
 
+// +kubebuilder:rbac:groups="",resources=configmaps,verbs=create;get;update
+
 func ReconcileConfigMap(owner metav1.Object, configMap *corev1.ConfigMap, reqLogger logr.Logger,
 	client controllerClient.Client, scheme *runtime.Scheme) (*corev1.ConfigMap, error) {
 	var err error
@@ -186,6 +192,8 @@ func ReconcileConfigMap(owner metav1.Object, configMap *corev1.ConfigMap, reqLog
 	return configMap, errorutil.WithMessagef(err, "error creating or updating ConfigMap %s/%s", configMap.Namespace, configMap.Name)
 }
 
+// +kubebuilder:rbac:groups="",resources=services,verbs=create;get;update
+
 func ReconcileService(owner metav1.Object, service *corev1.Service, reqLogger logr.Logger,
 	client controllerClient.Client, scheme *runtime.Scheme) (*corev1.Service, error) {
 	var err error

diff --git a/controllers/servicediscovery/servicediscovery_controller.go b/controllers/servicediscovery/servicediscovery_controller.go
@@ -103,6 +103,7 @@ type ServiceDiscoveryReconciler struct {
 
 // +kubebuilder:rbac:groups=submariner.io,resources=servicediscoveries,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=submariner.io,resources=servicediscoveries/status,verbs=get;update;patch
+
 func (r *ServiceDiscoveryReconciler) Reconcile(request reconcile.Request) (reconcile.Result, error) {
 	reqLogger := log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name)
 	reqLogger.Info("Reconciling ServiceDiscovery")
@@ -377,6 +378,9 @@ func newLighthouseCoreDNSService(cr *submarinerv1alpha1.ServiceDiscovery) *corev
 	}
 }
 
+// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;update
+// +kubebuilder:rbac:groups="",resources=services,verbs=get
+
 func updateDNSCustomConfigMap(client controllerClient.Client, k8sclientSet clientset.Interface, cr *submarinerv1alpha1.ServiceDiscovery,
 	reqLogger logr.Logger) error {
 	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
@@ -415,6 +419,9 @@ func updateDNSCustomConfigMap(client controllerClient.Client, k8sclientSet clien
 	return retryErr
 }
 
+// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;update
+// +kubebuilder:rbac:groups="",resources=services,verbs=get
+
 func updateDNSConfigMap(client controllerClient.Client, k8sclientSet clientset.Interface, cr *submarinerv1alpha1.ServiceDiscovery,
 	reqLogger logr.Logger) error {
 	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
@@ -556,6 +563,10 @@ func getImagePath(submariner *submarinerv1alpha1.ServiceDiscovery, componentImag
 		submariner.Spec.ImageOverrides)
 }
 
+// Resources need to be listable as well as watchable (for the informer)
+// +kubebuilder:rbac:groups=submariner.io,resources=servicediscoveries,verbs=list;watch
+// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=list;watch
+
 func (r *ServiceDiscoveryReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	// These are required so that we can manipulate DNS ConfigMap
 	if err := operatorv1.Install(mgr.GetScheme()); err != nil {

diff --git a/controllers/submariner/broker_controller.go b/controllers/submariner/broker_controller.go
@@ -42,10 +42,9 @@ type BrokerReconciler struct {
 	Scheme *runtime.Scheme
 }
 
-// TODO skitt: these rbac declarations (and others, see submariner_controller.go) need to be separated
-// from methods in order to be taken into account; but they produce ClusterRoles, not the Roles we want
 // +kubebuilder:rbac:groups=submariner.io,resources=brokers,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=submariner.io,resources=brokers/status,verbs=get;update;patch
+
 func (r *BrokerReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) {
 	_ = context.Background()
 	_ = r.Log.WithValues("broker", request.NamespacedName)

diff --git a/controllers/submariner/submariner_controller.go b/controllers/submariner/submariner_controller.go
@@ -105,6 +105,7 @@ type SubmarinerReconciler struct {
 
 // +kubebuilder:rbac:groups=submariner.io,resources=submariners,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=submariner.io,resources=submariners/status,verbs=get;update;patch
+
 func (r *SubmarinerReconciler) Reconcile(request reconcile.Request) (reconcile.Result, error) {
 	reqLogger := log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name)
 	reqLogger.Info("Reconciling Submariner")
@@ -277,6 +278,8 @@ func (r *SubmarinerReconciler) checkDaemonSetContainers(daemonSet *appsv1.Daemon
 	return mismatchedContainerImages, &nonReadyContainerStates, nil
 }
 
+// +kubebuilder:rbac:groups="",resources=pods,verbs=list
+
 func (r *SubmarinerReconciler) retrieveDaemonSetContainerStatuses(daemonSet *appsv1.DaemonSet,
 	namespace string) (*[]corev1.ContainerStatus, error) {
 	pods := &corev1.PodList{}
@@ -295,6 +298,8 @@ func (r *SubmarinerReconciler) retrieveDaemonSetContainerStatuses(daemonSet *app
 	return &containerStatuses, nil
 }
 
+// +kubebuilder:rbac:groups=submariner.io,resources=gateways,verbs=list
+
 func (r *SubmarinerReconciler) retrieveGateways(owner metav1.Object, namespace string) (*[]submv1.Gateway, error) {
 	foundGateways := &submv1.GatewayList{}
 	err := r.client.List(context.TODO(), foundGateways, client.InNamespace(namespace))
@@ -702,6 +707,10 @@ func getImagePath(submariner *submopv1a1.Submariner, componentImage string) stri
 		submariner.Spec.ImageOverrides)
 }
 
+// Resources need to be listable as well as watchable (for the informer)
+// +kubebuilder:rbac:groups=submariner.io,resources=gateways;submariners,verbs=list;watch
+// +kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=list;watch
+
 func (r *SubmarinerReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	// Set up the CRDs we need
 	crdUpdater, err := crdutils.NewFromRestConfig(mgr.GetConfig())