From 5037ade47a96ac89df085d9930d6ddbab57aa634 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Tue, 24 Sep 2024 16:23:31 +0300 Subject: [PATCH 01/16] Adding basic structure, need to write the logic --- cmd/kor/rolebindings.go | 30 ++++++ .../exceptions/rolebindings/rolebindings.json | 34 +++++++ pkg/kor/kor.go | 1 + pkg/kor/rolebindings.go | 96 +++++++++++++++++++ 4 files changed, 161 insertions(+) create mode 100644 cmd/kor/rolebindings.go create mode 100644 pkg/kor/exceptions/rolebindings/rolebindings.json create mode 100644 pkg/kor/rolebindings.go diff --git a/cmd/kor/rolebindings.go b/cmd/kor/rolebindings.go new file mode 100644 index 00000000..1e226612 --- /dev/null +++ b/cmd/kor/rolebindings.go @@ -0,0 +1,30 @@ +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{"rb", "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/exceptions/rolebindings/rolebindings.json b/pkg/kor/exceptions/rolebindings/rolebindings.json new file mode 100644 index 00000000..60aaead4 --- /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 + } + ] +} \ No newline at end of file diff --git a/pkg/kor/kor.go b/pkg/kor/kor.go index 2be6a72a..d3614a20 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 } diff --git a/pkg/kor/rolebindings.go b/pkg/kor/rolebindings.go new file mode 100644 index 00000000..0bc4a2d6 --- /dev/null +++ b/pkg/kor/rolebindings.go @@ -0,0 +1,96 @@ +package kor + +import ( + "bytes" + "context" + _ "embed" + "encoding/json" + "fmt" + "os" + + "github.com/yonahd/kor/pkg/common" + "github.com/yonahd/kor/pkg/filters" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +//go:embed exceptions/rolebindings/rolebindings.json +var roleBindingsConfig []byte + +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 + } + + 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 + } + + exceptionFound, err := isResourceException(rb.Name, rb.Namespace, config.ExceptionRoleBindings) + if err != nil { + return nil, err + } + + if exceptionFound { + continue + } + + // TODO write the logic to determine if this is unused + unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "d"}) + } + + 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 +} From ef5e41980848d376fec1b3b0e42f18fac2af3ac1 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Mon, 30 Sep 2024 09:50:30 +0300 Subject: [PATCH 02/16] #1 validate role exists --- pkg/kor/rolebindings.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/kor/rolebindings.go b/pkg/kor/rolebindings.go index 0bc4a2d6..f7de4baa 100644 --- a/pkg/kor/rolebindings.go +++ b/pkg/kor/rolebindings.go @@ -23,6 +23,16 @@ func processNamespaceRoleBindings(clientset kubernetes.Interface, namespace stri return nil, err } + roleList, err := clientset.RbacV1().Roles(namespace).List(context.TODO(), metav1.ListOptions{LabelSelector: filterOpts.IncludeLabels}) + if err != nil { + return nil, err + } + + roleNames := make(map[string]bool) + for _, role := range roleList.Items { + roleNames[role.Name] = true + } + config, err := unmarshalConfig(roleBindingsConfig) if err != nil { return nil, err @@ -44,8 +54,9 @@ func processNamespaceRoleBindings(clientset kubernetes.Interface, namespace stri continue } - // TODO write the logic to determine if this is unused - unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "d"}) + if !roleNames[rb.RoleRef.Name] { + unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "Role does not exists"}) + } } return unusedRoleBindingNames, nil From ad74c05cc25717be430cc7b1d1bf7168b44a8d78 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Mon, 30 Sep 2024 12:03:49 +0300 Subject: [PATCH 03/16] validating role ref exists, and adding basic tests --- pkg/kor/rolebindings.go | 21 ++++++++++- pkg/kor/rolebindings_test.go | 73 ++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 pkg/kor/rolebindings_test.go diff --git a/pkg/kor/rolebindings.go b/pkg/kor/rolebindings.go index f7de4baa..54d64496 100644 --- a/pkg/kor/rolebindings.go +++ b/pkg/kor/rolebindings.go @@ -33,6 +33,17 @@ func processNamespaceRoleBindings(clientset kubernetes.Interface, namespace stri roleNames[role.Name] = true } + // TODO is this list too big ? + clusterRoleList, err := clientset.RbacV1().ClusterRoles().List(context.TODO(), metav1.ListOptions{}) + if err != nil { + return nil, err + } + + clusterRoleNames := make(map[string]bool) + for _, cr := range clusterRoleList.Items { + clusterRoleNames[cr.Name] = true + } + config, err := unmarshalConfig(roleBindingsConfig) if err != nil { return nil, err @@ -54,8 +65,14 @@ func processNamespaceRoleBindings(clientset kubernetes.Interface, namespace stri continue } - if !roleNames[rb.RoleRef.Name] { - unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "Role does not exists"}) + if rb.RoleRef.Kind == "Role" && !roleNames[rb.RoleRef.Name] { + unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "Referenced Role does not exists"}) + continue + } + + if rb.RoleRef.Kind == "ClusterRole" && !clusterRoleNames[rb.RoleRef.Name] { + unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "Referenced Cluster role does not exists"}) + continue } } diff --git a/pkg/kor/rolebindings_test.go b/pkg/kor/rolebindings_test.go new file mode 100644 index 00000000..7c04d712 --- /dev/null +++ b/pkg/kor/rolebindings_test.go @@ -0,0 +1,73 @@ +package kor + +import ( + "context" + "testing" + + "github.com/yonahd/kor/pkg/filters" + 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" +) + +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) + } + + testRoleBindings1 := CreateTestRoleBinding( + testNamespace, + "role-ref-rb", + "test-rb-sa", + &rbacv1.RoleRef{ + Kind: "Role", + Name: "empty-role", + }) + _, err = clientset.RbacV1().RoleBindings(testNamespace).Create(context.TODO(), testRoleBindings1, v1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating fake %s: %v", "RoleBinding: testRoleBindings1", err) + } + + testRoleBindings2 := CreateTestRoleBinding( + testNamespace, + "cluster-role-ref-rb", + "test-rb-sa", + &rbacv1.RoleRef{ + Kind: "ClusterRole", + Name: "empty-cluster-rule", + }) + _, err = clientset.RbacV1().RoleBindings(testNamespace).Create(context.TODO(), testRoleBindings2, v1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating fake %s: %v", "RoleBinding: testRoleBindings2", 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) + } + + if len(unusedRoleBindings) != 2 { + t.Errorf("Expected 2 unused role bindings, got %d", len(unusedRoleBindings)) + } +} + +func init() { + scheme.Scheme = runtime.NewScheme() + _ = appsv1.AddToScheme(scheme.Scheme) +} From 5de51bf50d8d9d901fb55bff5437d701981d44e8 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Mon, 30 Sep 2024 14:54:47 +0300 Subject: [PATCH 04/16] handle service accounts --- pkg/kor/rolebindings.go | 74 +++++++++++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 18 deletions(-) diff --git a/pkg/kor/rolebindings.go b/pkg/kor/rolebindings.go index 54d64496..ff25f82d 100644 --- a/pkg/kor/rolebindings.go +++ b/pkg/kor/rolebindings.go @@ -10,6 +10,7 @@ import ( "github.com/yonahd/kor/pkg/common" "github.com/yonahd/kor/pkg/filters" + v1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) @@ -17,31 +18,59 @@ import ( //go:embed exceptions/rolebindings/rolebindings.json var roleBindingsConfig []byte -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}) +func createNamesMap(names []string, _ []string, err error) (map[string]bool, error) { if err != nil { return nil, err } - roleList, err := clientset.RbacV1().Roles(namespace).List(context.TODO(), metav1.ListOptions{LabelSelector: filterOpts.IncludeLabels}) + namesMap := make(map[string]bool) + for _, n := range names { + namesMap[n] = true + } + + return namesMap, nil +} + +// 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 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 := make(map[string]bool) - for _, role := range roleList.Items { - roleNames[role.Name] = true + roleNames, err := createNamesMap(retrieveRoleNames(clientset, namespace, filterOpts)) + if err != nil { + return nil, err } - // TODO is this list too big ? - clusterRoleList, err := clientset.RbacV1().ClusterRoles().List(context.TODO(), metav1.ListOptions{}) + clusterRoleNames, err := createNamesMap(retrieveClusterRoleNames(clientset, filterOpts)) if err != nil { return nil, err } - clusterRoleNames := make(map[string]bool) - for _, cr := range clusterRoleList.Items { - clusterRoleNames[cr.Name] = true + serviceAccountNames, err := createNamesMap(retrieveServiceAccountNames(clientset, namespace, filterOpts)) + if err != nil { + return nil, err } config, err := unmarshalConfig(roleBindingsConfig) @@ -56,24 +85,33 @@ func processNamespaceRoleBindings(clientset kubernetes.Interface, namespace stri continue } - exceptionFound, err := isResourceException(rb.Name, rb.Namespace, config.ExceptionRoleBindings) - if err != nil { + if exceptionFound, err := isResourceException(rb.Name, rb.Namespace, config.ExceptionRoleBindings); err != nil { return nil, err - } - - if exceptionFound { + } else if exceptionFound { continue } if rb.RoleRef.Kind == "Role" && !roleNames[rb.RoleRef.Name] { - unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "Referenced Role does not exists"}) + unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "RoleBinding references a non-existing Role"}) continue } if rb.RoleRef.Kind == "ClusterRole" && !clusterRoleNames[rb.RoleRef.Name] { - unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "Referenced Cluster role does not exists"}) + unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "RoleBinding references a non-existing ClusterRole"}) 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 From e03483e5038878b8d63cd9688348f2b8d4fbcb70 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Mon, 30 Sep 2024 15:50:45 +0300 Subject: [PATCH 05/16] more tests --- pkg/kor/rolebindings_test.go | 77 +++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 14 deletions(-) diff --git a/pkg/kor/rolebindings_test.go b/pkg/kor/rolebindings_test.go index 7c04d712..b6cec1d1 100644 --- a/pkg/kor/rolebindings_test.go +++ b/pkg/kor/rolebindings_test.go @@ -25,32 +25,72 @@ func createTestRoleBindings(t *testing.T) *fake.Clientset { t.Fatalf("Error creating namespace %s: %v", testNamespace, err) } - testRoleBindings1 := CreateTestRoleBinding( + rb1 := CreateTestRoleBinding( testNamespace, - "role-ref-rb", - "test-rb-sa", + "rb1", + "sa1", &rbacv1.RoleRef{ Kind: "Role", - Name: "empty-role", + Name: "non-exists-role", }) - _, err = clientset.RbacV1().RoleBindings(testNamespace).Create(context.TODO(), testRoleBindings1, v1.CreateOptions{}) + _, err = clientset.RbacV1().RoleBindings(testNamespace).Create(context.TODO(), rb1, v1.CreateOptions{}) if err != nil { - t.Fatalf("Error creating fake %s: %v", "RoleBinding: testRoleBindings1", err) + t.Fatalf("Error creating fake %s: %v", "RoleBinding: rb1", err) } - testRoleBindings2 := CreateTestRoleBinding( + rb2 := CreateTestRoleBinding( testNamespace, - "cluster-role-ref-rb", - "test-rb-sa", + "rb2", + "sa2", &rbacv1.RoleRef{ Kind: "ClusterRole", - Name: "empty-cluster-rule", + Name: "non-existing-cluster-rule", }) - _, err = clientset.RbacV1().RoleBindings(testNamespace).Create(context.TODO(), testRoleBindings2, v1.CreateOptions{}) + _, err = clientset.RbacV1().RoleBindings(testNamespace).Create(context.TODO(), rb2, v1.CreateOptions{}) if err != nil { - t.Fatalf("Error creating fake %s: %v", "RoleBinding: testRoleBindings2", err) + 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, + "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, + "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 } @@ -62,9 +102,18 @@ func TestProcessNamespaceRoleBindings(t *testing.T) { t.Errorf("Expected no error, got %v", err) } - if len(unusedRoleBindings) != 2 { - t.Errorf("Expected 2 unused role bindings, got %d", len(unusedRoleBindings)) + expectedRoleBindingNames := []string{"rb1", "rb2", "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 init() { From 310f84895474cf1f3ddce895a1a032ee463a0eca Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Mon, 30 Sep 2024 16:09:13 +0300 Subject: [PATCH 06/16] add to all arg --- pkg/kor/all.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) 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) } } From 58ba300dbcc5d693ef2d1f79fef4945f40959a75 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Mon, 30 Sep 2024 16:18:45 +0300 Subject: [PATCH 07/16] more tests --- pkg/kor/rolebindings_test.go | 50 ++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/pkg/kor/rolebindings_test.go b/pkg/kor/rolebindings_test.go index b6cec1d1..2b40cd5e 100644 --- a/pkg/kor/rolebindings_test.go +++ b/pkg/kor/rolebindings_test.go @@ -2,8 +2,11 @@ package kor import ( "context" + "encoding/json" + "reflect" "testing" + "github.com/yonahd/kor/pkg/common" "github.com/yonahd/kor/pkg/filters" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -27,7 +30,7 @@ func createTestRoleBindings(t *testing.T) *fake.Clientset { rb1 := CreateTestRoleBinding( testNamespace, - "rb1", + "test-rb1", "sa1", &rbacv1.RoleRef{ Kind: "Role", @@ -40,7 +43,7 @@ func createTestRoleBindings(t *testing.T) *fake.Clientset { rb2 := CreateTestRoleBinding( testNamespace, - "rb2", + "test-rb2", "sa2", &rbacv1.RoleRef{ Kind: "ClusterRole", @@ -59,7 +62,7 @@ func createTestRoleBindings(t *testing.T) *fake.Clientset { rb3 := CreateTestRoleBinding( testNamespace, - "rb3", + "test-rb3", "non-existing-service-account", &rbacv1.RoleRef{ Kind: "Role", @@ -72,7 +75,7 @@ func createTestRoleBindings(t *testing.T) *fake.Clientset { rb4 := CreateTestRoleBinding( testNamespace, - "rb4", + "test-rb4", "non-existing-service-account", &rbacv1.RoleRef{ Kind: "Role", @@ -102,7 +105,7 @@ func TestProcessNamespaceRoleBindings(t *testing.T) { t.Errorf("Expected no error, got %v", err) } - expectedRoleBindingNames := []string{"rb1", "rb2", "rb3"} + 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)) @@ -116,6 +119,43 @@ func TestProcessNamespaceRoleBindings(t *testing.T) { } +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) From 7333e33ccc54464d3aee346d03ef4fa4c3e47902 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Mon, 30 Sep 2024 16:31:37 +0300 Subject: [PATCH 08/16] delete.go --- pkg/kor/delete.go | 7 +++++++ 1 file changed, 7 insertions(+) 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) } From 8f3f180c49425f8652c38241fde066a3b0a123c8 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Mon, 30 Sep 2024 16:40:14 +0300 Subject: [PATCH 09/16] multi.go --- pkg/kor/multi.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/kor/multi.go b/pkg/kor/multi.go index 914ca6d5..60959203 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 "rb", "rolebinding", "rolebindings": + diffResult = getUnusedNetworkPolicies(clientset, namespace, filterOpts) default: fmt.Printf("resource type %q is not supported\n", resource) } From 3aa34225f14e586752d590cc38ea24843774fd3c Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Mon, 30 Sep 2024 17:00:14 +0300 Subject: [PATCH 10/16] readme --- README.md | 3 +++ 1 file changed, 3 insertions(+) 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 | | From 73e7c861ef7f0b9c82bf7eb456ee5fd28d4c8759 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Mon, 30 Sep 2024 17:11:10 +0300 Subject: [PATCH 11/16] refactor --- pkg/kor/exceptions/rolebindings/rolebindings.json | 2 +- pkg/kor/rolebindings.go | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/kor/exceptions/rolebindings/rolebindings.json b/pkg/kor/exceptions/rolebindings/rolebindings.json index 60aaead4..df1b8619 100644 --- a/pkg/kor/exceptions/rolebindings/rolebindings.json +++ b/pkg/kor/exceptions/rolebindings/rolebindings.json @@ -31,4 +31,4 @@ "MatchRegex": true } ] -} \ No newline at end of file +} diff --git a/pkg/kor/rolebindings.go b/pkg/kor/rolebindings.go index ff25f82d..f31a41fa 100644 --- a/pkg/kor/rolebindings.go +++ b/pkg/kor/rolebindings.go @@ -18,7 +18,8 @@ import ( //go:embed exceptions/rolebindings/rolebindings.json var roleBindingsConfig []byte -func createNamesMap(names []string, _ []string, err error) (map[string]bool, error) { +// 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 } @@ -58,17 +59,17 @@ func processNamespaceRoleBindings(clientset kubernetes.Interface, namespace stri return nil, err } - roleNames, err := createNamesMap(retrieveRoleNames(clientset, namespace, filterOpts)) + roleNames, err := convertNamesToPresenseMap(retrieveRoleNames(clientset, namespace, filterOpts)) if err != nil { return nil, err } - clusterRoleNames, err := createNamesMap(retrieveClusterRoleNames(clientset, filterOpts)) + clusterRoleNames, err := convertNamesToPresenseMap(retrieveClusterRoleNames(clientset, filterOpts)) if err != nil { return nil, err } - serviceAccountNames, err := createNamesMap(retrieveServiceAccountNames(clientset, namespace, filterOpts)) + serviceAccountNames, err := convertNamesToPresenseMap(retrieveServiceAccountNames(clientset, namespace, filterOpts)) if err != nil { return nil, err } From d1ac387a6a7207579476cd333203db504649386d Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Tue, 1 Oct 2024 08:52:02 +0300 Subject: [PATCH 12/16] import order --- pkg/kor/rolebindings.go | 5 +++-- pkg/kor/rolebindings_test.go | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/kor/rolebindings.go b/pkg/kor/rolebindings.go index f31a41fa..f15e76f1 100644 --- a/pkg/kor/rolebindings.go +++ b/pkg/kor/rolebindings.go @@ -8,11 +8,12 @@ import ( "fmt" "os" - "github.com/yonahd/kor/pkg/common" - "github.com/yonahd/kor/pkg/filters" 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 diff --git a/pkg/kor/rolebindings_test.go b/pkg/kor/rolebindings_test.go index 2b40cd5e..a865a7a9 100644 --- a/pkg/kor/rolebindings_test.go +++ b/pkg/kor/rolebindings_test.go @@ -6,8 +6,6 @@ import ( "reflect" "testing" - "github.com/yonahd/kor/pkg/common" - "github.com/yonahd/kor/pkg/filters" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -15,6 +13,9 @@ import ( "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 { From b0e97366f2366c373649e5790503f29996857289 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Tue, 1 Oct 2024 08:56:28 +0300 Subject: [PATCH 13/16] - --- cmd/kor/rolebindings.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/kor/rolebindings.go b/cmd/kor/rolebindings.go index 1e226612..d33f7217 100644 --- a/cmd/kor/rolebindings.go +++ b/cmd/kor/rolebindings.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/spf13/cobra" + "github.com/yonahd/kor/pkg/kor" "github.com/yonahd/kor/pkg/utils" ) From b8c0394dea71570cf6270dd891041931e3e3c09f Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Sun, 13 Oct 2024 17:38:04 +0300 Subject: [PATCH 14/16] Move convertNamesToPresenseMap to kor.go, and create checkRoleReferences function --- pkg/kor/kor.go | 14 ++++++++++++++ pkg/kor/rolebindings.go | 36 +++++++++++++++--------------------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/pkg/kor/kor.go b/pkg/kor/kor.go index d3614a20..be6968c0 100644 --- a/pkg/kor/kor.go +++ b/pkg/kor/kor.go @@ -191,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/rolebindings.go b/pkg/kor/rolebindings.go index f15e76f1..c6b33d27 100644 --- a/pkg/kor/rolebindings.go +++ b/pkg/kor/rolebindings.go @@ -19,20 +19,6 @@ import ( //go:embed exceptions/rolebindings/rolebindings.json var roleBindingsConfig []byte -// 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 -} - // 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 @@ -54,6 +40,18 @@ func isUsingValidServiceAccount(serviceAccounts []v1.Subject, serviceAccountName return false } +func checkRoleReferences(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 { @@ -93,13 +91,9 @@ func processNamespaceRoleBindings(clientset kubernetes.Interface, namespace stri continue } - if rb.RoleRef.Kind == "Role" && !roleNames[rb.RoleRef.Name] { - unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "RoleBinding references a non-existing Role"}) - continue - } - - if rb.RoleRef.Kind == "ClusterRole" && !clusterRoleNames[rb.RoleRef.Name] { - unusedRoleBindingNames = append(unusedRoleBindingNames, ResourceInfo{Name: rb.Name, Reason: "RoleBinding references a non-existing ClusterRole"}) + unusedRoleReference := checkRoleReferences(rb, roleNames, clusterRoleNames) + if unusedRoleReference != nil { + unusedRoleBindingNames = append(unusedRoleBindingNames, *unusedRoleReference) continue } From 4cad6862debcb40d2f6f380ce97df7de828e8a26 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Sun, 13 Oct 2024 17:42:03 +0300 Subject: [PATCH 15/16] remove rb --- cmd/kor/rolebindings.go | 2 +- pkg/kor/multi.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/kor/rolebindings.go b/cmd/kor/rolebindings.go index d33f7217..5b471450 100644 --- a/cmd/kor/rolebindings.go +++ b/cmd/kor/rolebindings.go @@ -11,7 +11,7 @@ import ( var roleBindingCmd = &cobra.Command{ Use: "rolebinding", - Aliases: []string{"rb", "rolebindings"}, + Aliases: []string{"rolebindings"}, Short: "Gets unused role bindings", Args: cobra.ExactArgs(0), Run: func(cmd *cobra.Command, args []string) { diff --git a/pkg/kor/multi.go b/pkg/kor/multi.go index 60959203..ef2dfd94 100644 --- a/pkg/kor/multi.go +++ b/pkg/kor/multi.go @@ -89,7 +89,7 @@ func retrieveNamespaceDiffs(clientset kubernetes.Interface, namespace string, re diffResult = getUnusedDaemonSets(clientset, namespace, filterOpts) case "netpol", "networkpolicy", "networkpolicies": diffResult = getUnusedNetworkPolicies(clientset, namespace, filterOpts) - case "rb", "rolebinding", "rolebindings": + case "rolebinding", "rolebindings": diffResult = getUnusedNetworkPolicies(clientset, namespace, filterOpts) default: fmt.Printf("resource type %q is not supported\n", resource) From b8d9cf83d91275dc031a9738c85d3ffc37493b12 Mon Sep 17 00:00:00 2001 From: nati-elmaliach Date: Sun, 13 Oct 2024 17:53:36 +0300 Subject: [PATCH 16/16] naming --- pkg/kor/rolebindings.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/kor/rolebindings.go b/pkg/kor/rolebindings.go index c6b33d27..a9b13291 100644 --- a/pkg/kor/rolebindings.go +++ b/pkg/kor/rolebindings.go @@ -40,7 +40,7 @@ func isUsingValidServiceAccount(serviceAccounts []v1.Subject, serviceAccountName return false } -func checkRoleReferences(rb v1.RoleBinding, roleNames, clusterRoleNames map[string]bool) *ResourceInfo { +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"} } @@ -91,9 +91,9 @@ func processNamespaceRoleBindings(clientset kubernetes.Interface, namespace stri continue } - unusedRoleReference := checkRoleReferences(rb, roleNames, clusterRoleNames) - if unusedRoleReference != nil { - unusedRoleBindingNames = append(unusedRoleBindingNames, *unusedRoleReference) + roleReferenceIssue := validateRoleReference(rb, roleNames, clusterRoleNames) + if roleReferenceIssue != nil { + unusedRoleBindingNames = append(unusedRoleBindingNames, *roleReferenceIssue) continue }