From 257ad3ec4f34c45b8a152f12323bc4986b94512f Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Wed, 24 Feb 2021 10:07:43 +0100 Subject: [PATCH] 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: #1105 Signed-off-by: Stephen Kitt --- Makefile | 2 +- config/rbac/submariner-operator/role.yaml | 206 +++++++++++++----- .../servicediscovery_controller.go | 7 + controllers/submariner/broker_controller.go | 3 +- .../submariner/submariner_controller.go | 1 + pkg/broker/ensure.go | 6 + pkg/broker/globalcidr_cm.go | 6 + pkg/broker/rbac.go | 3 + pkg/discovery/network/canal.go | 2 + pkg/discovery/network/generic.go | 4 + pkg/discovery/network/ovnkubernetes.go | 3 + pkg/discovery/network/pods.go | 2 + .../operator/common/namespace/ensure.go | 2 + pkg/utils/createorupdate.go | 14 ++ 14 files changed, 201 insertions(+), 60 deletions(-) diff --git a/Makefile b/Makefile index 600f607f0a..bba7922446 100644 --- 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 index 2e44cd617a..5533d53365 100644 --- a/config/rbac/submariner-operator/role.yaml +++ b/config/rbac/submariner-operator/role.yaml @@ -1,63 +1,155 @@ + --- 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: - - '*' +- resources: + - configmaps + verbs: + - create + - get + - update +- resources: + - namespaces + verbs: + - create +- resources: + - nodes + verbs: + - list +- resources: + - pods + verbs: + - list +- resources: + - secrets + verbs: + - get +- resources: + - serviceaccounts + verbs: + - create + - get + - update +- resources: + - services + verbs: + - create + - get +- apiGroups: + - apiextensions.k8s.io + resources: + - customresourcedefinitions + verbs: + - create + - get + - update +- apiGroups: + - apps + resources: + - deployments + verbs: + - create + - get + - update +- 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: + - rbac + resources: + - serviceaccounts + verbs: + - create +- 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: + - 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/servicediscovery/servicediscovery_controller.go b/controllers/servicediscovery/servicediscovery_controller.go index 20c2814b30..e32d605a3e 100644 --- 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:resources=configmaps,verbs=get;update +// +kubebuilder:rbac: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:resources=configmaps,verbs=get;update +// +kubebuilder:rbac: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 { diff --git a/controllers/submariner/broker_controller.go b/controllers/submariner/broker_controller.go index aeb4c6b82a..be84e6c00e 100644 --- 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 index 302834f5ae..ecb17b5119 100644 --- 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") diff --git a/pkg/broker/ensure.go b/pkg/broker/ensure.go index a41627c62d..67c827861c 100644 --- a/pkg/broker/ensure.go +++ b/pkg/broker/ensure.go @@ -177,6 +177,8 @@ func WaitForClientToken(clientset *kubernetes.Clientset, submarinerBrokerSA stri return secret, err } +// +kubebuilder:rbac:resources=namespaces,verbs=create + func CreateNewBrokerNamespace(clientset *kubernetes.Clientset) (brokernamespace *v1.Namespace, err error) { return clientset.CoreV1().Namespaces().Create(NewBrokerNamespace()) } @@ -189,6 +191,8 @@ func CreateOrUpdateBrokerAdminRole(clientset *kubernetes.Clientset) (created boo return utils.CreateOrUpdateRole(clientset, SubmarinerBrokerNamespace, NewBrokerAdminRole()) } +// +kubebuilder:rbac:groups=rbac,resources=serviceaccounts,verbs=create + func CreateNewBrokerRoleBinding(clientset *kubernetes.Clientset, serviceAccount, role string) (brokerRoleBinding *rbac.RoleBinding, err error) { return clientset.RbacV1().RoleBindings(SubmarinerBrokerNamespace).Create( @@ -196,6 +200,8 @@ func CreateNewBrokerRoleBinding(clientset *kubernetes.Clientset, serviceAccount, ) } +// +kubebuilder:rbac:resources=serviceaccounts,verbs=create + func CreateNewBrokerSA(clientset *kubernetes.Clientset, submarinerBrokerSA string) (brokerSA *v1.ServiceAccount, err error) { return clientset.CoreV1().ServiceAccounts(SubmarinerBrokerNamespace).Create(NewBrokerSA(submarinerBrokerSA)) } diff --git a/pkg/broker/globalcidr_cm.go b/pkg/broker/globalcidr_cm.go index 834c4b9d96..0883d2eb3c 100644 --- a/pkg/broker/globalcidr_cm.go +++ b/pkg/broker/globalcidr_cm.go @@ -39,6 +39,8 @@ type ClusterInfo struct { GlobalCidr []string `json:"global_cidr"` } +// +kubebuilder:rbac:resources=configmaps,verbs=create + func CreateGlobalnetConfigMap(config *rest.Config, globalnetEnabled bool, defaultGlobalCidrRange string, defaultGlobalClusterSize uint, namespace string) error { clientset, err := kubernetes.NewForConfig(config) @@ -95,6 +97,8 @@ func NewGlobalnetConfigMap(globalnetEnabled bool, defaultGlobalCidrRange string, return cm, nil } +// +kubebuilder:rbac:resources=configmaps,verbs=update + func UpdateGlobalnetConfigMap(k8sClientset *kubernetes.Clientset, namespace string, configMap *v1.ConfigMap, newCluster ClusterInfo) error { var clusterInfo []ClusterInfo @@ -128,6 +132,8 @@ func UpdateGlobalnetConfigMap(k8sClientset *kubernetes.Clientset, namespace stri return err } +// +kubebuilder:rbac:resources=configmaps,verbs=get + func GetGlobalnetConfigMap(k8sClientset *kubernetes.Clientset, namespace string) (*v1.ConfigMap, error) { cm, err := k8sClientset.CoreV1().ConfigMaps(namespace).Get(GlobalCIDRConfigMapName, metav1.GetOptions{}) if err != nil { diff --git a/pkg/broker/rbac.go b/pkg/broker/rbac.go index cc1b9c090b..dd01d60db6 100644 --- a/pkg/broker/rbac.go +++ b/pkg/broker/rbac.go @@ -129,6 +129,9 @@ func NewBrokerRoleBinding(serviceAccount, role string) *rbacv1.RoleBinding { return binding } +// +kubebuilder:rbac:resources=serviceaccounts,verbs=get +// +kubebuilder:rbac:resources=secrets,verbs=get + func GetClientTokenSecret(clientSet clientset.Interface, brokerNamespace, submarinerBrokerSA string) (*v1.Secret, error) { sa, err := clientSet.CoreV1().ServiceAccounts(brokerNamespace).Get(submarinerBrokerSA, metav1.GetOptions{}) if err != nil { diff --git a/pkg/discovery/network/canal.go b/pkg/discovery/network/canal.go index ed253519da..ec9a95ec1d 100644 --- a/pkg/discovery/network/canal.go +++ b/pkg/discovery/network/canal.go @@ -26,6 +26,8 @@ import ( "k8s.io/client-go/kubernetes" ) +// +kubebuilder:rbac:resources=configmaps,verbs=get + func discoverCanalFlannelNetwork(clientSet kubernetes.Interface) (*ClusterNetwork, error) { // TODO: this must be smarter, looking for the canal daemonset, with labels k8s-app=canal // and then the reference on the container volumes: diff --git a/pkg/discovery/network/generic.go b/pkg/discovery/network/generic.go index 8d9753cb78..1e58e61d73 100644 --- a/pkg/discovery/network/generic.go +++ b/pkg/discovery/network/generic.go @@ -76,6 +76,8 @@ func findClusterIPRangeFromApiserver(clientSet kubernetes.Interface) (string, er return findPodCommandParameter(clientSet, "component=kube-apiserver", "--service-cluster-ip-range") } +// +kubebuilder:rbac:resources=services,verbs=create + func findClusterIPRangeFromServiceCreation(clientSet kubernetes.Interface) (string, error) { // find service cidr based on https://stackoverflow.com/questions/44190607/how-do-you-find-the-cluster-service-cidr-of-a-kubernetes-cluster invalidSvcSpec := &v1.Service{ @@ -160,6 +162,8 @@ func findPodIPRangeKubeProxy(clientSet kubernetes.Interface) (string, error) { return findPodCommandParameter(clientSet, "component=kube-proxy", "--cluster-cidr") } +// +kubebuilder:rbac:resources=nodes,verbs=list + func findPodIPRangeFromNodeSpec(clientSet kubernetes.Interface) (string, error) { nodes, err := clientSet.CoreV1().Nodes().List(v1meta.ListOptions{}) diff --git a/pkg/discovery/network/ovnkubernetes.go b/pkg/discovery/network/ovnkubernetes.go index f3091b41d2..43f5b199fb 100644 --- a/pkg/discovery/network/ovnkubernetes.go +++ b/pkg/discovery/network/ovnkubernetes.go @@ -33,6 +33,9 @@ const ( OvnKubernetes = "OVNKubernetes" ) +// +kubebuilder:rbac:resources=services,verbs=get +// +kubebuilder:rbac:resources=configmaps,verbs=get + func discoverOvnKubernetesNetwork(clientSet kubernetes.Interface) (*ClusterNetwork, error) { ovnDBPod, err := findPod(clientSet, "name=ovnkube-db") diff --git a/pkg/discovery/network/pods.go b/pkg/discovery/network/pods.go index dafc881b43..4ff79d451b 100644 --- a/pkg/discovery/network/pods.go +++ b/pkg/discovery/network/pods.go @@ -49,6 +49,8 @@ func findPodCommandParameter(clientSet kubernetes.Interface, labelSelector, para return "", nil } +// +kubebuilder:rbac:resources=pods,verbs=list + func findPod(clientSet kubernetes.Interface, labelSelector string) (*v1.Pod, error) { pods, err := clientSet.CoreV1().Pods("").List(v1meta.ListOptions{ LabelSelector: labelSelector, diff --git a/pkg/subctl/operator/common/namespace/ensure.go b/pkg/subctl/operator/common/namespace/ensure.go index b3e8ed0b9d..2e1248cbca 100644 --- a/pkg/subctl/operator/common/namespace/ensure.go +++ b/pkg/subctl/operator/common/namespace/ensure.go @@ -24,6 +24,8 @@ import ( "k8s.io/client-go/rest" ) +// +kubebuilder:rbac:resources=namespaces,verbs=create + // Ensure functions updates or installs the operator CRDs in the cluster func Ensure(restConfig *rest.Config, namespace string) (bool, error) { clientSet, err := clientset.NewForConfig(restConfig) diff --git a/pkg/utils/createorupdate.go b/pkg/utils/createorupdate.go index 8c464da122..c35aace029 100644 --- a/pkg/utils/createorupdate.go +++ b/pkg/utils/createorupdate.go @@ -32,6 +32,8 @@ import ( crdutils "github.com/submariner-io/submariner-operator/pkg/utils/crds" ) +// +kubebuilder:rbac:groups=rbac,resources=clusterroles,verbs=create;get;update + func CreateOrUpdateClusterRole(clientSet clientset.Interface, clusterRole *rbacv1.ClusterRole) (bool, error) { _, err := clientSet.RbacV1().ClusterRoles().Create(clusterRole) if err == nil { @@ -52,6 +54,8 @@ func CreateOrUpdateClusterRole(clientSet clientset.Interface, clusterRole *rbacv return false, err } +// +kubebuilder:rbac:groups=rbac,resources=clusterrolebindings,verbs=create;get;update + func CreateOrUpdateClusterRoleBinding(clientSet clientset.Interface, clusterRoleBinding *rbacv1.ClusterRoleBinding) (bool, error) { _, err := clientSet.RbacV1().ClusterRoleBindings().Create(clusterRoleBinding) if err == nil { @@ -72,6 +76,8 @@ func CreateOrUpdateClusterRoleBinding(clientSet clientset.Interface, clusterRole return false, err } +// +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=create;get;update + func CreateOrUpdateCRD(updater crdutils.CRDUpdater, crd *apiextensions.CustomResourceDefinition) (bool, error) { _, err := updater.Create(crd) if err == nil { @@ -102,6 +108,8 @@ func CreateOrUpdateEmbeddedCRD(updater crdutils.CRDUpdater, crdYaml string) (boo return CreateOrUpdateCRD(updater, crd) } +// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;get;update + func CreateOrUpdateDeployment(clientSet clientset.Interface, namespace string, deployment *appsv1.Deployment) (bool, error) { _, err := clientSet.AppsV1().Deployments(namespace).Create(deployment) if err != nil && errors.IsAlreadyExists(err) { @@ -120,6 +128,8 @@ func CreateOrUpdateDeployment(clientSet clientset.Interface, namespace string, d return true, err } +// +kubebuilder:rbac:groups=rbac,resources=roles,verbs=create;get;update + func CreateOrUpdateRole(clientSet clientset.Interface, namespace string, role *rbacv1.Role) (bool, error) { _, err := clientSet.RbacV1().Roles(namespace).Create(role) if err != nil && errors.IsAlreadyExists(err) { @@ -138,6 +148,8 @@ func CreateOrUpdateRole(clientSet clientset.Interface, namespace string, role *r return true, err } +// +kubebuilder:rbac:groups=rbac,resources=rolebindings,verbs=create;get;update + func CreateOrUpdateRoleBinding(clientSet clientset.Interface, namespace string, roleBinding *rbacv1.RoleBinding) (bool, error) { _, err := clientSet.RbacV1().RoleBindings(namespace).Create(roleBinding) if err != nil && errors.IsAlreadyExists(err) { @@ -156,6 +168,8 @@ func CreateOrUpdateRoleBinding(clientSet clientset.Interface, namespace string, return true, err } +// +kubebuilder:rbac:resources=serviceaccounts,verbs=create;get;update + func CreateOrUpdateServiceAccount(clientSet clientset.Interface, namespace string, sa *corev1.ServiceAccount) (bool, error) { _, err := clientSet.CoreV1().ServiceAccounts(namespace).Create(sa) if err != nil && errors.IsAlreadyExists(err) {