From 673c21e337dace7c37cc404327b192c5140ad929 Mon Sep 17 00:00:00 2001 From: nati-elmaliach <45143653+nati-elmaliach@users.noreply.github.com> Date: Sun, 13 Oct 2024 21:53:38 +0300 Subject: [PATCH] Feat: Discover unused RoleBindings (#362) * Adding basic structure, need to write the logic * #1 validate role exists * validating role ref exists, and adding basic tests * handle service accounts * more tests * add to all arg * more tests * delete.go * multi.go * readme * refactor * import order * - * Move convertNamesToPresenseMap to kor.go, and create checkRoleReferences function * remove rb * naming --- README.md | 3 + cmd/kor/rolebindings.go | 31 ++++ pkg/kor/all.go | 15 ++ pkg/kor/delete.go | 7 + .../exceptions/rolebindings/rolebindings.json | 34 ++++ pkg/kor/kor.go | 15 ++ pkg/kor/multi.go | 2 + pkg/kor/rolebindings.go | 158 +++++++++++++++++ pkg/kor/rolebindings_test.go | 163 ++++++++++++++++++ 9 files changed, 428 insertions(+) create mode 100644 cmd/kor/rolebindings.go create mode 100644 pkg/kor/exceptions/rolebindings/rolebindings.json create mode 100644 pkg/kor/rolebindings.go create mode 100644 pkg/kor/rolebindings_test.go diff --git a/README.md b/README.md index 46e14125..31aec8e9 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ Kor is a tool to discover unused Kubernetes resources. Currently, Kor can identi - DaemonSets - StorageClasses - NetworkPolicies +- RoleBindings ![Kor Screenshot](/images/show_reason_screenshot.png) @@ -110,6 +111,7 @@ Kor provides various subcommands to identify and list unused resources. The avai - `statefulset` - Gets unused StatefulSets for the specified namespace or all namespaces. - `role` - Gets unused Roles for the specified namespace or all namespaces. - `clusterrole` - Gets unused ClusterRoles for the specified namespace or all namespaces (namespace refers to RoleBinding). +- `rolebinding` - Gets unused RoleBindings for the specified namespace or all namespaces. - `hpa` - Gets unused HPAs for the specified namespace or all namespaces. - `pod` - Gets unused Pods for the specified namespace or all namespaces. - `pvc` - Gets unused PVCs for the specified namespace or all namespaces. @@ -172,6 +174,7 @@ kor [subcommand] --help | StatefulSets | Statefulsets with no Replicas | | | Roles | Roles not used in roleBinding | | | ClusterRoles | ClusterRoles not used in roleBinding or clusterRoleBinding
ClusterRoles not used in ClusterRole aggregation | | +| RoleBindings | RoleBindings referencing invalid Role, ClusterRole, or ServiceAccounts | | | PVCs | PVCs not used in Pods | | | Ingresses | Ingresses not pointing at any Service | | | Hpas | HPAs not used in Deployments
HPAs not used in StatefulSets | | diff --git a/cmd/kor/rolebindings.go b/cmd/kor/rolebindings.go new file mode 100644 index 00000000..5b471450 --- /dev/null +++ b/cmd/kor/rolebindings.go @@ -0,0 +1,31 @@ +package kor + +import ( + "fmt" + + "github.com/spf13/cobra" + + "github.com/yonahd/kor/pkg/kor" + "github.com/yonahd/kor/pkg/utils" +) + +var roleBindingCmd = &cobra.Command{ + Use: "rolebinding", + Aliases: []string{"rolebindings"}, + Short: "Gets unused role bindings", + Args: cobra.ExactArgs(0), + Run: func(cmd *cobra.Command, args []string) { + clientset := kor.GetKubeClient(kubeconfig) + + if response, err := kor.GetUnusedRoleBindings(filterOptions, clientset, outputFormat, opts); err != nil { + fmt.Println(err) + } else { + utils.PrintLogo(outputFormat) + fmt.Println(response) + } + }, +} + +func init() { + rootCmd.AddCommand(roleBindingCmd) +} diff --git a/pkg/kor/all.go b/pkg/kor/all.go index 37af741f..ffaad22b 100644 --- a/pkg/kor/all.go +++ b/pkg/kor/all.go @@ -264,6 +264,19 @@ func getUnusedNetworkPolicies(clientset kubernetes.Interface, namespace string, return namespaceNetpolDiff } +func getUnusedRoleBindings(clientset kubernetes.Interface, namespace string, filterOpts *filters.Options) ResourceDiff { + roleBindingDiff, err := processNamespaceRoleBindings(clientset, namespace, filterOpts) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to get %s namespace %s: %v\n", "RoleBindings", namespace, err) + } + + namespaceRoleBindingDiff := ResourceDiff{ + "RoleBinding", + roleBindingDiff, + } + return namespaceRoleBindingDiff +} + func GetUnusedAllNamespaced(filterOpts *filters.Options, clientset kubernetes.Interface, outputFormat string, opts common.Opts) (string, error) { resources := make(map[string]map[string][]ResourceInfo) for _, namespace := range filterOpts.Namespaces(clientset) { @@ -286,6 +299,7 @@ func GetUnusedAllNamespaced(filterOpts *filters.Options, clientset kubernetes.In resources[namespace]["ReplicaSet"] = getUnusedReplicaSets(clientset, namespace, filterOpts).diff resources[namespace]["DaemonSet"] = getUnusedDaemonSets(clientset, namespace, filterOpts).diff resources[namespace]["NetworkPolicy"] = getUnusedNetworkPolicies(clientset, namespace, filterOpts).diff + resources[namespace]["RoleBinding"] = getUnusedRoleBindings(clientset, namespace, filterOpts).diff case "resource": appendResources(resources, "ConfigMap", namespace, getUnusedCMs(clientset, namespace, filterOpts).diff) appendResources(resources, "Service", namespace, getUnusedSVCs(clientset, namespace, filterOpts).diff) @@ -303,6 +317,7 @@ func GetUnusedAllNamespaced(filterOpts *filters.Options, clientset kubernetes.In appendResources(resources, "ReplicaSet", namespace, getUnusedReplicaSets(clientset, namespace, filterOpts).diff) appendResources(resources, "DaemonSet", namespace, getUnusedDaemonSets(clientset, namespace, filterOpts).diff) appendResources(resources, "NetworkPolicy", namespace, getUnusedNetworkPolicies(clientset, namespace, filterOpts).diff) + appendResources(resources, "RoleBinding", namespace, getUnusedRoleBindings(clientset, namespace, filterOpts).diff) } } diff --git a/pkg/kor/delete.go b/pkg/kor/delete.go index a1e6f687..7efcec3e 100644 --- a/pkg/kor/delete.go +++ b/pkg/kor/delete.go @@ -81,6 +81,9 @@ func DeleteResourceCmd() map[string]func(clientset kubernetes.Interface, namespa "NetworkPolicy": func(clientset kubernetes.Interface, namespace, name string) error { return clientset.NetworkingV1().NetworkPolicies(namespace).Delete(context.TODO(), name, metav1.DeleteOptions{}) }, + "RoleBinding": func(clientset kubernetes.Interface, namespace, name string) error { + return clientset.RbacV1().RoleBindings(namespace).Delete(context.TODO(), name, metav1.DeleteOptions{}) + }, } return deleteResourceApiMap @@ -170,6 +173,8 @@ func updateResource(clientset kubernetes.Interface, namespace, resourceType stri return clientset.StorageV1().StorageClasses().Update(context.TODO(), resource.(*storagev1.StorageClass), metav1.UpdateOptions{}) case "NetworkPolicy": return clientset.NetworkingV1().NetworkPolicies(namespace).Update(context.TODO(), resource.(*networkingv1.NetworkPolicy), metav1.UpdateOptions{}) + case "RoleBinding": + return clientset.RbacV1().RoleBindings(namespace).Update(context.TODO(), resource.(*rbacv1.RoleBinding), metav1.UpdateOptions{}) } return nil, fmt.Errorf("resource type '%s' is not supported", resourceType) } @@ -214,6 +219,8 @@ func getResource(clientset kubernetes.Interface, namespace, resourceType, resour return clientset.StorageV1().StorageClasses().Get(context.TODO(), resourceName, metav1.GetOptions{}) case "NetworkPolicy": return clientset.NetworkingV1().NetworkPolicies(namespace).Get(context.TODO(), resourceName, metav1.GetOptions{}) + case "RoleBinding": + return clientset.RbacV1().RoleBindings(namespace).Get(context.TODO(), resourceName, metav1.GetOptions{}) } return nil, fmt.Errorf("resource type '%s' is not supported", resourceType) } diff --git a/pkg/kor/exceptions/rolebindings/rolebindings.json b/pkg/kor/exceptions/rolebindings/rolebindings.json new file mode 100644 index 00000000..df1b8619 --- /dev/null +++ b/pkg/kor/exceptions/rolebindings/rolebindings.json @@ -0,0 +1,34 @@ +{ + "exceptionRoleBindings": [ + { + "Namespace": "kube-public", + "ResourceName": "kubeadm:bootstrap-signer-clusterinfo" + }, + { + "Namespace": "kube-public", + "ResourceName": "system:controller:bootstrap-signer" + }, + { + "Namespace": "kube-system", + "ResourceName": "kube-proxy" + }, + { + "Namespace": "kube-system", + "ResourceName": "kubeadm:kubelet-config" + }, + { + "Namespace": "kube-system", + "ResourceName": "kubeadm:nodes-kubeadm-config" + }, + { + "Namespace": "kube-system", + "ResourceName": "system::*", + "MatchRegex": true + }, + { + "Namespace": "kube-system", + "ResourceName": "system:controller:*", + "MatchRegex": true + } + ] +} diff --git a/pkg/kor/kor.go b/pkg/kor/kor.go index 2be6a72a..be6968c0 100644 --- a/pkg/kor/kor.go +++ b/pkg/kor/kor.go @@ -38,6 +38,7 @@ type Config struct { ExceptionStorageClasses []ExceptionResource `json:"exceptionStorageClasses"` ExceptionJobs []ExceptionResource `json:"exceptionJobs"` ExceptionPdbs []ExceptionResource `json:"exceptionPdbs"` + ExceptionRoleBindings []ExceptionResource `json:"exceptionRoleBindings"` // Add other configurations if needed } @@ -190,3 +191,17 @@ func resourceInfoContains(slice []ResourceInfo, item string) bool { } return false } + +// Convert a slice of names into a map for fast lookup +func convertNamesToPresenseMap(names []string, _ []string, err error) (map[string]bool, error) { + if err != nil { + return nil, err + } + + namesMap := make(map[string]bool) + for _, n := range names { + namesMap[n] = true + } + + return namesMap, nil +} diff --git a/pkg/kor/multi.go b/pkg/kor/multi.go index 914ca6d5..ef2dfd94 100644 --- a/pkg/kor/multi.go +++ b/pkg/kor/multi.go @@ -89,6 +89,8 @@ func retrieveNamespaceDiffs(clientset kubernetes.Interface, namespace string, re diffResult = getUnusedDaemonSets(clientset, namespace, filterOpts) case "netpol", "networkpolicy", "networkpolicies": diffResult = getUnusedNetworkPolicies(clientset, namespace, filterOpts) + case "rolebinding", "rolebindings": + diffResult = getUnusedNetworkPolicies(clientset, namespace, filterOpts) default: fmt.Printf("resource type %q is not supported\n", resource) } diff --git a/pkg/kor/rolebindings.go b/pkg/kor/rolebindings.go new file mode 100644 index 00000000..a9b13291 --- /dev/null +++ b/pkg/kor/rolebindings.go @@ -0,0 +1,158 @@ +package kor + +import ( + "bytes" + "context" + _ "embed" + "encoding/json" + "fmt" + "os" + + v1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + + "github.com/yonahd/kor/pkg/common" + "github.com/yonahd/kor/pkg/filters" +) + +//go:embed exceptions/rolebindings/rolebindings.json +var roleBindingsConfig []byte + +// Filter out subjects base on Kind, can be later used for User and Group +func filterSubjects(subjects []v1.Subject, kind string) []v1.Subject { + var serviceAccountSubjects []v1.Subject + for _, subject := range subjects { + if subject.Kind == kind { + serviceAccountSubjects = append(serviceAccountSubjects, subject) + } + } + return serviceAccountSubjects +} + +// Check if any valid service accounts exist in the RoleBinding +func isUsingValidServiceAccount(serviceAccounts []v1.Subject, serviceAccountNames map[string]bool) bool { + for _, sa := range serviceAccounts { + if serviceAccountNames[sa.Name] { + return true + } + } + return false +} + +func validateRoleReference(rb v1.RoleBinding, roleNames, clusterRoleNames map[string]bool) *ResourceInfo { + if rb.RoleRef.Kind == "Role" && !roleNames[rb.RoleRef.Name] { + return &ResourceInfo{Name: rb.Name, Reason: "RoleBinding references a non-existing Role"} + } + + if rb.RoleRef.Kind == "ClusterRole" && !clusterRoleNames[rb.RoleRef.Name] { + return &ResourceInfo{Name: rb.Name, Reason: "RoleBinding references a non-existing ClusterRole"} + } + + return nil +} + +func processNamespaceRoleBindings(clientset kubernetes.Interface, namespace string, filterOpts *filters.Options) ([]ResourceInfo, error) { + roleBindingsList, err := clientset.RbacV1().RoleBindings(namespace).List(context.TODO(), metav1.ListOptions{LabelSelector: filterOpts.IncludeLabels}) + if err != nil { + return nil, err + } + + roleNames, err := convertNamesToPresenseMap(retrieveRoleNames(clientset, namespace, filterOpts)) + if err != nil { + return nil, err + } + + clusterRoleNames, err := convertNamesToPresenseMap(retrieveClusterRoleNames(clientset, filterOpts)) + if err != nil { + return nil, err + } + + serviceAccountNames, err := convertNamesToPresenseMap(retrieveServiceAccountNames(clientset, namespace, filterOpts)) + if err != nil { + return nil, err + } + + config, err := unmarshalConfig(roleBindingsConfig) + if err != nil { + return nil, err + } + + var unusedRoleBindingNames []ResourceInfo + + for _, rb := range roleBindingsList.Items { + if pass, _ := filter.SetObject(&rb).Run(filterOpts); pass { + continue + } + + if exceptionFound, err := isResourceException(rb.Name, rb.Namespace, config.ExceptionRoleBindings); err != nil { + return nil, err + } else if exceptionFound { + continue + } + + roleReferenceIssue := validateRoleReference(rb, roleNames, clusterRoleNames) + if roleReferenceIssue != nil { + unusedRoleBindingNames = append(unusedRoleBindingNames, *roleReferenceIssue) + continue + } + + serviceAccountSubjects := filterSubjects(rb.Subjects, "ServiceAccount") + + // If other kinds (Users/Groups) are used, we assume they exists for now + if len(serviceAccountSubjects) != len(rb.Subjects) { + continue + } + + // Check if RoleBinding uses a valid service account + if !isUsingValidServiceAccount(serviceAccountSubjects, serviceAccountNames) { + unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "RoleBinding references a non-existing ServiceAccount"}) + } + } + + return unusedRoleBindingNames, nil +} + +func GetUnusedRoleBindings(filterOpts *filters.Options, clientset kubernetes.Interface, outputFormat string, opts common.Opts) (string, error) { + resources := make(map[string]map[string][]ResourceInfo) + for _, namespace := range filterOpts.Namespaces(clientset) { + diff, err := processNamespaceRoleBindings(clientset, namespace, filterOpts) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to process namespace %s: %v\n", namespace, err) + continue + } + + if opts.DeleteFlag { + if diff, err = DeleteResource(diff, clientset, namespace, "RoleBinding", opts.NoInteractive); err != nil { + fmt.Fprintf(os.Stderr, "Failed to delete RoleBinding %s in namespace %s: %v\n", diff, namespace, err) + } + } + + switch opts.GroupBy { + case "namespace": + resources[namespace] = make(map[string][]ResourceInfo) + resources[namespace]["RoleBinding"] = diff + case "resource": + appendResources(resources, "RoleBinding", namespace, diff) + } + } + + var outputBuffer bytes.Buffer + var jsonResponse []byte + switch outputFormat { + case "table": + outputBuffer = FormatOutput(resources, opts) + case "json", "yaml": + var err error + if jsonResponse, err = json.MarshalIndent(resources, "", " "); err != nil { + return "", err + } + } + + unusedRoleBindings, err := unusedResourceFormatter(outputFormat, outputBuffer, opts, jsonResponse) + if err != nil { + fmt.Printf("err: %v\n", err) + } + + return unusedRoleBindings, nil +} diff --git a/pkg/kor/rolebindings_test.go b/pkg/kor/rolebindings_test.go new file mode 100644 index 00000000..a865a7a9 --- /dev/null +++ b/pkg/kor/rolebindings_test.go @@ -0,0 +1,163 @@ +package kor + +import ( + "context" + "encoding/json" + "reflect" + "testing" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/kubernetes/scheme" + + "github.com/yonahd/kor/pkg/common" + "github.com/yonahd/kor/pkg/filters" +) + +func createTestRoleBindings(t *testing.T) *fake.Clientset { + clientset := fake.NewSimpleClientset() + + _, err := clientset.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{ + ObjectMeta: v1.ObjectMeta{Name: testNamespace}, + }, v1.CreateOptions{}) + + if err != nil { + t.Fatalf("Error creating namespace %s: %v", testNamespace, err) + } + + rb1 := CreateTestRoleBinding( + testNamespace, + "test-rb1", + "sa1", + &rbacv1.RoleRef{ + Kind: "Role", + Name: "non-exists-role", + }) + _, err = clientset.RbacV1().RoleBindings(testNamespace).Create(context.TODO(), rb1, v1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating fake %s: %v", "RoleBinding: rb1", err) + } + + rb2 := CreateTestRoleBinding( + testNamespace, + "test-rb2", + "sa2", + &rbacv1.RoleRef{ + Kind: "ClusterRole", + Name: "non-existing-cluster-rule", + }) + _, err = clientset.RbacV1().RoleBindings(testNamespace).Create(context.TODO(), rb2, v1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating fake %s: %v", "RoleBinding: rb2", err) + } + + testRole := CreateTestRole(testNamespace, "existing-role", AppLabels) + _, err = clientset.RbacV1().Roles(testNamespace).Create(context.TODO(), testRole, v1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating fake %s: %v", "Role", err) + } + + rb3 := CreateTestRoleBinding( + testNamespace, + "test-rb3", + "non-existing-service-account", + &rbacv1.RoleRef{ + Kind: "Role", + Name: "existing-role", + }) + _, err = clientset.RbacV1().RoleBindings(testNamespace).Create(context.TODO(), rb3, v1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating fake %s: %v", "RoleBinding: rb3", err) + } + + rb4 := CreateTestRoleBinding( + testNamespace, + "test-rb4", + "non-existing-service-account", + &rbacv1.RoleRef{ + Kind: "Role", + Name: "existing-role", + }) + + sa4 := CreateTestServiceAccount(testNamespace, "existing-service-account", AppLabels) + _, err = clientset.CoreV1().ServiceAccounts(testNamespace).Create(context.TODO(), sa4, v1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating fake %s: %v", "ServiceAccount", err) + } + + rbacSubject := CreateTestRbacSubject(testNamespace, "existing-service-account") + rb4.Subjects = append(rb4.Subjects, *rbacSubject) + _, err = clientset.RbacV1().RoleBindings(testNamespace).Create(context.TODO(), rb4, v1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating fake %s: %v", "RoleBinding: rb4", err) + } + return clientset +} + +func TestProcessNamespaceRoleBindings(t *testing.T) { + clientset := createTestRoleBindings(t) + + unusedRoleBindings, err := processNamespaceRoleBindings(clientset, testNamespace, &filters.Options{}) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + expectedRoleBindingNames := []string{"test-rb1", "test-rb2", "test-rb3"} + + if len(unusedRoleBindings) != len(expectedRoleBindingNames) { + t.Errorf("Expected %d unused role bindings, got %d", len(expectedRoleBindingNames), len(unusedRoleBindings)) + } + + for i, rb := range unusedRoleBindings { + if rb.Name != expectedRoleBindingNames[i] { + t.Errorf("Expected %s, got %s", expectedRoleBindingNames[i], rb.Name) + } + } + +} + +func TestGetUnusedRoleBindingStructured(t *testing.T) { + clientset := createTestRoleBindings(t) + + opts := common.Opts{ + WebhookURL: "", + Channel: "", + Token: "", + DeleteFlag: false, + NoInteractive: true, + GroupBy: "namespace", + } + + output, err := GetUnusedRoleBindings(&filters.Options{}, clientset, "json", opts) + if err != nil { + t.Fatalf("Error calling GetUnusedRoleBindingStructured: %v", err) + } + + expectedOutput := map[string]map[string][]string{ + testNamespace: { + "RoleBinding": { + "test-rb1", + "test-rb2", + "test-rb3", + }, + }, + } + + var actualOutput map[string]map[string][]string + if err := json.Unmarshal([]byte(output), &actualOutput); err != nil { + t.Fatalf("Error unmarshaling actual output: %v", err) + } + + if !reflect.DeepEqual(expectedOutput, actualOutput) { + t.Errorf("Expected output does not match actual output") + } +} + +func init() { + scheme.Scheme = runtime.NewScheme() + _ = appsv1.AddToScheme(scheme.Scheme) +}