Repository: yonahd/kor
Pull request #221 (2024-03-08 14:38:13)

fix: ClusterRole Aggregation
Kubernetes Supports ClusterRole Aggregation, enabling the aggregation of one ClusterRole into another. For more details, refer to the official Kubernetes Documentation [here](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#aggregated-clusterroles).

This functionality allows a ClusterRole lacking a ClusterRoleBinding or a RoleBinding to remain usable. Currently, Kor does not handle this scenario. Consequently, the ClusterRoles in use are not properly listed through Kor. The proposed Pull Request addresses this issue.

yonahd: Hi @sempex
Thank for the pr. Can you please add a test for this? 
sempex: > Hi @sempex Thank for the pr. Can you please add a test for this?

Sure will try to add one 👍 
sempex: @yonahd I added my Changed to the Testcases 
codecov-commenter: ## [Codecov](https://app.codecov.io/gh/yonahd/kor/pull/221?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) Report
Attention: Patch coverage is `60.60606%` with `13 lines` in your changes are missing coverage. Please review.
> Project coverage is 47.58%. Comparing base [(`c421565`)](https://app.codecov.io/gh/yonahd/kor/commit/c42156554991c644ef728626ccd27bd8cdaa219c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) to head [(`df57396`)](https://app.codecov.io/gh/yonahd/kor/pull/221?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen).

| [Files](https://app.codecov.io/gh/yonahd/kor/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) | Patch % | Lines |
|---|---|---|
| [pkg/kor/clusterroles.go](https://app.codecov.io/gh/yonahd/kor/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen#diff-cGtnL2tvci9jbHVzdGVycm9sZXMuZ28=) | 55.17% | [8 Missing and 5 partials :warning: ](https://app.codecov.io/gh/yonahd/kor/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) |

:exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #221      +/-   ##
==========================================
+ Coverage   47.48%   47.58%   +0.09%     
==========================================
  Files          32       32              
  Lines        2883     2915      +32     
==========================================
+ Hits         1369     1387      +18     
- Misses       1345     1354       +9     
- Partials      169      174       +5     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/yonahd/kor/pull/221?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen).

sempex: > Looks good! A couple small comments

Fixed it 🚀 

Patch:
diff --git a/.gitignore b/.gitignore
index e26e600..24efe2c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,4 @@
+.vscode
 *.iml
 .idea/
 dist/**
diff --git a/go.mod b/go.mod
index 8d3fa94..ea0eff7 100644
--- a/go.mod
+++ b/go.mod
@@ -7,6 +7,7 @@ require (
 	github.com/olekukonko/tablewriter v0.0.5
 	github.com/prometheus/client_golang v1.19.0
 	github.com/spf13/cobra v1.8.0
+	golang.org/x/exp v0.0.0-20240222234643-814bf88cf225
 	k8s.io/api v0.29.2
 	k8s.io/apiextensions-apiserver v0.29.2
 	k8s.io/apimachinery v0.29.2
diff --git a/go.sum b/go.sum
index bbb62f3..daebd4e 100644
--- a/go.sum
+++ b/go.sum
@@ -120,6 +120,8 @@ github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9dec
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/exp v0.0.0-20240222234643-814bf88cf225 h1:LfspQV/FYTatPTr/3HzIcmiUFH7PGP+OQ6mgDYo3yuQ=
+golang.org/x/exp v0.0.0-20240222234643-814bf88cf225/go.mod h1:CxmFvTBINI24O/j8iY7H1xHzx2i4OsyguNBmN/uPtqc=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
@@ -154,8 +156,8 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
-golang.org/x/tools v0.16.1 h1:TLyB3WofjdOEepBHAU20JdNC1Zbg87elYofWYAY5oZA=
-golang.org/x/tools v0.16.1/go.mod h1:kYVVN6I1mBNoB1OX+noeBjbRk4IUEPa7JJ+TJMEooJ0=
+golang.org/x/tools v0.18.0 h1:k8NLag8AGHnn+PHbl7g43CtqZAwG60vZkLqgyZgIHgQ=
+golang.org/x/tools v0.18.0/go.mod h1:GL7B4CwcLLeo59yx/9UWWuNOW1n3VZ4f5axWfML7Lcg=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/pkg/kor/clusterroles.go b/pkg/kor/clusterroles.go
index cf28017..2146bcc 100644
--- a/pkg/kor/clusterroles.go
+++ b/pkg/kor/clusterroles.go
@@ -5,9 +5,12 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"os"
+	"strconv"
+
 	"github.com/yonahd/kor/pkg/filters"
 	v1 "k8s.io/api/rbac/v1"
-	"os"
+	"k8s.io/utils/strings/slices"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -58,10 +61,51 @@ func retrieveUsedClusterRoles(clientset kubernetes.Interface, filterOpts *filter
 			continue
 		}
 		usedClusterRoles[crb.RoleRef.Name] = true
+	}
 
-		usedClusterRoles[crb.RoleRef.Name] = true
+	// Get a list of all ClusterRoles
+	clusterRoles, err := clientset.RbacV1().ClusterRoles().List(context.TODO(), metav1.ListOptions{})
+	if err != nil {
+		return nil, fmt.Errorf("failed to list cluster roles %v", err)
 	}
+	// Convert the ClusterRole list into a Map
+	clusterRolesMap := make(map[string]v1.ClusterRole)
+	for _, clusterRole := range clusterRoles.Items {
+		clusterRolesMap[clusterRole.Name] = clusterRole
+	}
+	// Create a list wich holds all aggregated labels
+	aggregatedLabels := make([]string, 0)
 
+	for clusterRole := range usedClusterRoles {
+		clusterRoleManifest := clusterRolesMap[clusterRole]
+		if clusterRolesMap[clusterRole].AggregationRule == nil {
+			continue
+		}
+		for _, label := range clusterRoleManifest.AggregationRule.ClusterRoleSelectors {
+			for key, value := range label.MatchLabels {
+				aggregatedLabels = append(aggregatedLabels, fmt.Sprintf("%s: %s", key, value))
+			}
+		}
+
+		for _, clusterRole := range clusterRoles.Items {
+			for label, value := range clusterRole.Labels {
+				if slices.Contains(aggregatedLabels, label+": "+value) {
+					usedClusterRoles[clusterRole.Name], err = strconv.ParseBool(value)
+					if err != nil {
+						return nil, fmt.Errorf("couldn't convert string to bool %v", err)
+					}
+					if clusterRole.AggregationRule == nil {
+						continue
+					}
+					for _, label := range clusterRole.AggregationRule.ClusterRoleSelectors {
+						for key, value := range label.MatchLabels {
+							aggregatedLabels = append(aggregatedLabels, key+": "+value)
+						}
+					}
+				}
+			}
+		}
+	}
 	var usedClusterRoleNames []string
 	for role := range usedClusterRoles {
 		usedClusterRoleNames = append(usedClusterRoleNames, role)


Test patch:
diff --git a/pkg/kor/clusterroles_test.go b/pkg/kor/clusterroles_test.go
index 1d720de..79a59d9 100644
--- a/pkg/kor/clusterroles_test.go
+++ b/pkg/kor/clusterroles_test.go
@@ -3,10 +3,12 @@ package kor
 import (
 	"context"
 	"encoding/json"
-	"github.com/yonahd/kor/pkg/filters"
 	"reflect"
+	"sort"
 	"testing"
 
+	"github.com/yonahd/kor/pkg/filters"
+
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -18,6 +20,10 @@ import (
 func createTestClusterRoles(t *testing.T) *fake.Clientset {
 	clientset := fake.NewSimpleClientset()
 
+	var AggregatedLabels = map[string]string{"rbac.authorization.k8s.io/aggregate-to-test-clusterRole1": "true"}
+	var matchLabels = v1.LabelSelector{
+		MatchLabels: AggregatedLabels,
+	}
 	_, err := clientset.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{
 		ObjectMeta: v1.ObjectMeta{Name: testNamespace},
 	}, v1.CreateOptions{})
@@ -32,7 +38,7 @@ func createTestClusterRoles(t *testing.T) *fake.Clientset {
 		t.Fatalf("Error creating fake %s: %v", "clusterRole", err)
 	}
 
-	clusterRole2 := CreateTestClusterRole("test-clusterRole2", AppLabels)
+	clusterRole2 := CreateTestClusterRole("test-clusterRole2", AppLabels, matchLabels)
 	_, err = clientset.RbacV1().ClusterRoles().Create(context.TODO(), clusterRole2, v1.CreateOptions{})
 	if err != nil {
 		t.Fatalf("Error creating fake %s: %v", "clusterRole", err)
@@ -56,6 +62,12 @@ func createTestClusterRoles(t *testing.T) *fake.Clientset {
 		t.Fatalf("Error creating fake %s: %v", "Role", err)
 	}
 
+	clusterRole6 := CreateTestClusterRole("test-clusterRole6", AggregatedLabels)
+	_, err = clientset.RbacV1().ClusterRoles().Create(context.TODO(), clusterRole6, v1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Error creating fake %s: %v", "Role", err)
+	}
+
 	testRoleRef2 := CreateTestRoleRefForClusterRole("test-clusterRole2")
 	testClusterRoleBinding := CreateTestClusterRoleBindingRoleRef(testNamespace, "test-rb2", "test-sa", testRoleRef2)
 	_, err = clientset.RbacV1().ClusterRoleBindings().Create(context.TODO(), testClusterRoleBinding, v1.CreateOptions{})
@@ -79,14 +91,15 @@ func TestRetrieveUsedClusterRoles(t *testing.T) {
 	if err != nil {
 		t.Errorf("Expected no error, got %v", err)
 	}
-
-	if len(usedClusterRoles) != 2 {
-		t.Errorf("Expected 2 used cluster role, got %d", len(usedClusterRoles))
+	if len(usedClusterRoles) != 3 {
+		t.Errorf("Expected 3 used cluster role, got %d", len(usedClusterRoles))
 	}
 
-	expectedRoles := []string{"test-clusterRole1", "test-clusterRole3", "test-clusterRole4"}
-	if reflect.DeepEqual(usedClusterRoles, expectedRoles) {
-		t.Errorf("Expected 'test-role1', 'test-role3', 'test-role4', got %s, %s, %s", usedClusterRoles[0], usedClusterRoles[1], usedClusterRoles[2])
+	expectedRoles := []string{"test-clusterRole2", "test-clusterRole3", "test-clusterRole6"}
+	sort.Strings(usedClusterRoles)
+	t.Log(usedClusterRoles)
+	if !reflect.DeepEqual(usedClusterRoles, expectedRoles) {
+		t.Errorf("Expected 'test-role3', 'test-role2', 'test-role6', got %s, %s, %s", usedClusterRoles[0], usedClusterRoles[1], usedClusterRoles[2])
 	}
 }
 
@@ -96,9 +109,8 @@ func TestRetrieveClusterRoleNames(t *testing.T) {
 	if err != nil {
 		t.Errorf("Expected no error, got %v", err)
 	}
-
-	if len(allRoles) != 3 {
-		t.Errorf("Expected 3 roles, got %d", len(allRoles))
+	if len(allRoles) != 4 {
+		t.Errorf("Expected 4 roles, got %d", len(allRoles))
 	}
 }
 
diff --git a/pkg/kor/create_test_resources.go b/pkg/kor/create_test_resources.go
index 449cef6..55a4b7e 100644
--- a/pkg/kor/create_test_resources.go
+++ b/pkg/kor/create_test_resources.go
@@ -14,7 +14,6 @@ import (
 )
 
 var testNamespace = "test-namespace"
-
 var AppLabels = map[string]string{}
 var UsedLabels = map[string]string{"kor/used": "true"}
 var UnusedLabels = map[string]string{"kor/used": "false"}
@@ -341,13 +340,16 @@ func CreateTestReplicaSet(namespace, name string, specReplicas *int32, status *a
 	}
 }
 
-func CreateTestClusterRole(name string, labels map[string]string) *rbacv1.ClusterRole {
+func CreateTestClusterRole(name string, labels map[string]string, matchLabels ...v1.LabelSelector) *rbacv1.ClusterRole {
 	policyRule := createPolicyRule()
 	return &rbacv1.ClusterRole{
 		ObjectMeta: v1.ObjectMeta{
 			Name:   name,
 			Labels: labels,
 		},
+		AggregationRule: &rbacv1.AggregationRule{
+			ClusterRoleSelectors: matchLabels,
+		},
 		Rules: []rbacv1.PolicyRule{*policyRule},
 	}
 }