chore: narrow down the rbac permissions for schedualer (#2024)

In this PR: - move destinations and collectorsgroup rbac permission to be role (on odigos ns) instead of clusterrole. These live anyway only in our ns, but it looks better in rbac reviews. - removed unused create/delete/update/patch for the destinations, as we only read them. - currently the leader election role brings in configmaps permissions, but I also added the permissions we need for the reconcilers so not to mix unrelated things. we will need to also understand and address the leader election role later on. - removed the finalizers permissions, as we don't use finalizers and it's unused. - created some consts in cli files for better structure to the references - synced helm files with the changes - tested both helm and cli locally to make sure it works InstrumentationConfig is left as a clusterrole, since they belong to the various namespaces where the sources reside.
odigos-io · Dec 19, 2024 · 371c5ab · 371c5ab
1 parent c6ff525
commit 371c5ab
Show file tree

Hide file tree

Showing 5 changed files with 145 additions and 69 deletions.
diff --git a/cli/cmd/resources/scheduler.go b/cli/cmd/resources/scheduler.go
@@ -7,6 +7,7 @@ import (
 	"github.com/odigos-io/odigos/cli/pkg/containers"
 	"github.com/odigos-io/odigos/cli/pkg/kube"
 	"github.com/odigos-io/odigos/common"
+	"github.com/odigos-io/odigos/common/consts"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -17,11 +18,16 @@ import (
 )
 
 const (
-	SchedulerImage          = "keyval/odigos-scheduler"
-	SchedulerServiceName    = "scheduler"
-	SchedulerDeploymentName = "odigos-scheduler"
-	SchedulerAppLabelValue  = "odigos-scheduler"
-	SchedulerContainerName  = "manager"
+	SchedulerImage                  = "keyval/odigos-scheduler"
+	SchedulerServiceName            = "scheduler"
+	SchedulerDeploymentName         = "odigos-scheduler"
+	SchedulerAppLabelValue          = SchedulerDeploymentName
+	SchedulerRoleName               = SchedulerDeploymentName
+	SchedulerRoleBindingName        = SchedulerDeploymentName
+	SchedulerClusterRoleName        = SchedulerDeploymentName
+	SchedulerClusterRoleBindingName = SchedulerDeploymentName
+	SchedulerServiceAccountName     = SchedulerDeploymentName
+	SchedulerContainerName          = "manager"
 )
 
 func NewSchedulerServiceAccount(ns string) *corev1.ServiceAccount {
@@ -31,13 +37,13 @@ func NewSchedulerServiceAccount(ns string) *corev1.ServiceAccount {
 			APIVersion: "v1",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "odigos-scheduler",
+			Name:      SchedulerServiceAccountName,
 			Namespace: ns,
 		},
 	}
 }
 
-func NewSchedulerRoleBinding(ns string) *rbacv1.RoleBinding {
+func NewSchedulerLeaderElectionRoleBinding(ns string) *rbacv1.RoleBinding {
 	return &rbacv1.RoleBinding{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "RoleBinding",
@@ -50,7 +56,7 @@ func NewSchedulerRoleBinding(ns string) *rbacv1.RoleBinding {
 		Subjects: []rbacv1.Subject{
 			{
 				Kind: "ServiceAccount",
-				Name: "odigos-scheduler",
+				Name: SchedulerServiceAccountName,
 			},
 		},
 		RoleRef: rbacv1.RoleRef{
@@ -61,42 +67,48 @@ func NewSchedulerRoleBinding(ns string) *rbacv1.RoleBinding {
 	}
 }
 
-func NewSchedulerClusterRole() *rbacv1.ClusterRole {
-	return &rbacv1.ClusterRole{
+func NewSchedulerRole(ns string) *rbacv1.Role {
+	return &rbacv1.Role{
 		TypeMeta: metav1.TypeMeta{
-			Kind:       "ClusterRole",
+			Kind:       "Role",
 			APIVersion: "rbac.authorization.k8s.io/v1",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "odigos-scheduler",
+			Name:      SchedulerRoleName,
+			Namespace: ns,
 		},
 		Rules: []rbacv1.PolicyRule{
 			{
 				Verbs: []string{
-					"create",
-					"delete",
 					"get",
 					"list",
-					"patch",
-					"update",
 					"watch",
 				},
 				APIGroups: []string{
-					"odigos.io",
+					"",
 				},
 				Resources: []string{
-					"collectorsgroups",
+					"configmaps",
+				},
+				ResourceNames: []string{
+					consts.OdigosConfigurationName,
 				},
 			},
 			{
 				Verbs: []string{
+					"create",
+					"delete",
+					"get",
+					"list",
+					"patch",
 					"update",
+					"watch",
 				},
 				APIGroups: []string{
 					"odigos.io",
 				},
 				Resources: []string{
-					"collectorsgroups/finalizers",
+					"collectorsgroups",
 				},
 			},
 			{
@@ -114,12 +126,8 @@ func NewSchedulerClusterRole() *rbacv1.ClusterRole {
 			},
 			{
 				Verbs: []string{
-					"create",
-					"delete",
 					"get",
 					"list",
-					"patch",
-					"update",
 					"watch",
 				},
 				APIGroups: []string{
@@ -129,17 +137,6 @@ func NewSchedulerClusterRole() *rbacv1.ClusterRole {
 					"destinations",
 				},
 			},
-			{
-				Verbs: []string{
-					"update",
-				},
-				APIGroups: []string{
-					"odigos.io",
-				},
-				Resources: []string{
-					"destinations/finalizers",
-				},
-			},
 			{
 				Verbs: []string{
 					"get",
@@ -153,6 +150,44 @@ func NewSchedulerClusterRole() *rbacv1.ClusterRole {
 					"destinations/status",
 				},
 			},
+		},
+	}
+}
+
+func NewSchedulerRoleBinding(ns string) *rbacv1.RoleBinding {
+	return &rbacv1.RoleBinding{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "RoleBinding",
+			APIVersion: "rbac.authorization.k8s.io/v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      SchedulerRoleBindingName,
+			Namespace: ns,
+		},
+		Subjects: []rbacv1.Subject{
+			{
+				Kind: "ServiceAccount",
+				Name: SchedulerServiceAccountName,
+			},
+		},
+		RoleRef: rbacv1.RoleRef{
+			APIGroup: "rbac.authorization.k8s.io",
+			Kind:     "Role",
+			Name:     SchedulerRoleName,
+		},
+	}
+}
+
+func NewSchedulerClusterRole() *rbacv1.ClusterRole {
+	return &rbacv1.ClusterRole{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "ClusterRole",
+			APIVersion: "rbac.authorization.k8s.io/v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: SchedulerClusterRoleName,
+		},
+		Rules: []rbacv1.PolicyRule{
 			{
 				Verbs: []string{
 					"list",
@@ -173,19 +208,19 @@ func NewSchedulerClusterRoleBinding(ns string) *rbacv1.ClusterRoleBinding {
 			APIVersion: "rbac.authorization.k8s.io/v1",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "odigos-scheduler",
+			Name: SchedulerClusterRoleBindingName,
 		},
 		Subjects: []rbacv1.Subject{
 			{
 				Kind:      "ServiceAccount",
-				Name:      "odigos-scheduler",
+				Name:      SchedulerServiceAccountName,
 				Namespace: ns,
 			},
 		},
 		RoleRef: rbacv1.RoleRef{
 			APIGroup: "rbac.authorization.k8s.io",
 			Kind:     "ClusterRole",
-			Name:     "odigos-scheduler",
+			Name:     SchedulerClusterRoleName,
 		},
 	}
 }
@@ -285,7 +320,7 @@ func NewSchedulerDeployment(ns string, version string, imagePrefix string) *apps
 						},
 					},
 					TerminationGracePeriodSeconds: ptrint64(10),
-					ServiceAccountName:            "odigos-scheduler",
+					ServiceAccountName:            SchedulerServiceAccountName,
 					SecurityContext: &corev1.PodSecurityContext{
 						RunAsNonRoot: ptrbool(true),
 					},
@@ -313,6 +348,8 @@ func (a *schedulerResourceManager) Name() string { return "Scheduler" }
 func (a *schedulerResourceManager) InstallFromScratch(ctx context.Context) error {
 	resources := []kube.Object{
 		NewSchedulerServiceAccount(a.ns),
+		NewSchedulerLeaderElectionRoleBinding(a.ns),
+		NewSchedulerRole(a.ns),
 		NewSchedulerRoleBinding(a.ns),
 		NewSchedulerClusterRole(),
 		NewSchedulerClusterRoleBinding(a.ns),

diff --git a/helm/odigos/templates/scheduler/clusterrole.yaml b/helm/odigos/templates/scheduler/clusterrole.yaml
@@ -3,40 +3,11 @@ kind: ClusterRole
 metadata:
   name: odigos-scheduler
 rules:
-  - apiGroups:
-      - odigos.io
-    resources:
-      - collectorsgroups
-      - destinations
-    verbs:
-      - create
-      - delete
-      - get
-      - list
-      - patch
-      - update
-      - watch
-  - apiGroups:
-      - odigos.io
-    resources:
-      - collectorsgroups/finalizers
-      - destinations/finalizers
-    verbs:
-      - update
-  - apiGroups:
-      - odigos.io
-    resources:
-      - collectorsgroups/status
-      - destinations/status
-    verbs:
-      - get
-      - patch
-      - update
   - apiGroups:
       - odigos.io
     resources:
       - instrumentationconfigs
     verbs:
-      - get
       - list
-      - watch
+      - get
+      - watch
diff --git a/helm/odigos/templates/scheduler/role-binding.yaml b/helm/odigos/templates/scheduler/role-binding.yaml
@@ -0,0 +1,11 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: odigos-scheduler
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: odigos-scheduler
+subjects:
+  - kind: ServiceAccount
+    name: odigos-scheduler
diff --git a/helm/odigos/templates/scheduler/role.yaml b/helm/odigos/templates/scheduler/role.yaml
@@ -0,0 +1,51 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: odigos-scheduler
+rules:
+  - apiGroups:
+    - ""
+    resourceNames:
+    - odigos-config
+    resources:
+    - configmaps
+    verbs:
+    - get
+    - list
+    - watch
+  - apiGroups:
+    - odigos.io
+    resources:
+    - collectorsgroups
+    verbs:
+    - create
+    - delete
+    - get
+    - list
+    - patch
+    - update
+    - watch
+  - apiGroups:
+    - odigos.io
+    resources:
+    - collectorsgroups/status
+    verbs:
+    - get
+    - patch
+    - update
+  - apiGroups:
+    - odigos.io
+    resources:
+    - destinations
+    verbs:
+    - get
+    - list
+    - watch
+  - apiGroups:
+    - odigos.io
+    resources:
+    - destinations/status
+    verbs:
+    - get
+    - patch
+    - update
diff --git a/scheduler/main.go b/scheduler/main.go
@@ -94,6 +94,12 @@ func main() {
 				&corev1.ConfigMap{}: {
 					Field: odigosConfigSelector,
 				},
+				&odigosv1.CollectorsGroup{}: {
+					Field: nsSelector,
+				},
+				&odigosv1.Destination{}: {
+					Field: nsSelector,
+				},
 			},
 		},
 		HealthProbeBindAddress: probeAddr,