chore: address review comments

Author: yashmehrotra

scrapers/kubernetes/rbac.go

@@ -313,6 +313,9 @@ func (r *rbacExtractor) processRoleBinding(obj *unstructured.Unstructured) {
 	// Find all target resources based on the rules
 	targetResources := r.findTargetResources(rules, bindingNamespace, kind == "ClusterRoleBinding")
 
+	// Compute the role alias once for the binding
+	roleAlias := KubernetesAlias(r.clusterName, roleKind, roleNamespace, roleName)
+
 	// Get subjects
 	subjects, found, _ := unstructured.NestedSlice(obj.Object, "subjects")
 	if !found {
@@ -387,11 +390,21 @@ func (r *rbacExtractor) processRoleBinding(obj *unstructured.Unstructured) {
 		// If we have rules and target resources, create ConfigAccess for each resource
 		if hasRules && len(targetResources) > 0 {
 			for _, target := range targetResources {
+				subjectAlias := userAlias
+				if subjectAlias == "" {
+					subjectAlias = groupAlias
+				}
+				targetExternalID := KubernetesAlias(r.clusterName, target.kind, target.namespace, target.name)
+
 				access := v1.ExternalConfigAccess{
+					ConfigAccess: models.ConfigAccess{
+						ID: generateRBACID(subjectAlias, targetExternalID, roleAlias).String(),
+					},
 					ConfigExternalID: v1.ExternalID{
-						ExternalID: KubernetesAlias(r.clusterName, target.kind, target.namespace, target.name),
+						ExternalID: targetExternalID,
 						ConfigType: GetConfigTypeForKind(target.kind),
 					},
+					ExternalRoleAliases: []string{roleAlias},
 				}
 
 				if userAlias != "" {

scrapers/kubernetes/rbac_test.go

@@ -96,10 +96,14 @@ func TestRBACExtractor_ProcessRoleBinding_ServiceAccount(t *testing.T) {
 	access := extractor.getAccess()
 	require.Len(t, access, 2, "expected 2 config access entries (one per pod in namespace)")
 
+	expectedRoleAlias := KubernetesAlias(clusterName, "Role", "default", "pod-reader")
+
 	// Check that access entries point to pods, not the role
 	for _, a := range access {
 		assert.Equal(t, ConfigTypePrefix+"Pod", a.ConfigExternalID.ConfigType)
 		assert.Equal(t, []string{expectedUserAlias}, a.ExternalUserAliases)
+		assert.Equal(t, []string{expectedRoleAlias}, a.ExternalRoleAliases)
+		assert.NotEmpty(t, a.ID, "access ID should be set")
 	}
 }
 
@@ -136,6 +140,12 @@ func TestRBACExtractor_ProcessRoleBinding_User(t *testing.T) {
 	// Should have ConfigAccess for the pod and service (cluster-wide)
 	access := extractor.getAccess()
 	assert.Len(t, access, 2, "expected 2 config access entries (pod + service)")
+
+	expectedRoleAlias := KubernetesAlias(clusterName, "ClusterRole", "", "cluster-admin")
+	for _, a := range access {
+		assert.Equal(t, []string{expectedRoleAlias}, a.ExternalRoleAliases)
+		assert.NotEmpty(t, a.ID, "access ID should be set")
+	}
 }
 
 func TestRBACExtractor_ProcessRoleBinding_Group(t *testing.T) {
@@ -171,6 +181,10 @@ func TestRBACExtractor_ProcessRoleBinding_Group(t *testing.T) {
 	require.Len(t, access, 1, "expected 1 config access entry")
 	assert.Empty(t, access[0].ExternalUserAliases)
 	assert.Equal(t, []string{expectedGroupAlias}, access[0].ExternalGroupAliases)
+
+	expectedRoleAlias := KubernetesAlias(clusterName, "ClusterRole", "", "view")
+	assert.Equal(t, []string{expectedRoleAlias}, access[0].ExternalRoleAliases)
+	assert.NotEmpty(t, access[0].ID, "access ID should be set")
 }
 
 func TestRBACExtractor_ProcessRoleBinding_MixedSubjects(t *testing.T) {
@@ -203,6 +217,12 @@ func TestRBACExtractor_ProcessRoleBinding_MixedSubjects(t *testing.T) {
 	// Each subject gets one ConfigAccess entry for the pod
 	access := extractor.getAccess()
 	assert.Len(t, access, 3, "expected 3 config access entries (one per subject, all pointing to same pod)")
+
+	expectedRoleAlias := KubernetesAlias(clusterName, "ClusterRole", "", "edit")
+	for _, a := range access {
+		assert.Equal(t, []string{expectedRoleAlias}, a.ExternalRoleAliases)
+		assert.NotEmpty(t, a.ID, "access ID should be set")
+	}
 }
 
 func TestRBACExtractor_Deduplication(t *testing.T) {
@@ -286,6 +306,10 @@ func TestRBACExtractor_CRDResourceResolution(t *testing.T) {
 	require.Len(t, access, 1, "expected 1 config access entry for the canary instance")
 	assert.Equal(t, ConfigTypePrefix+"Canary", access[0].ConfigExternalID.ConfigType)
 	assert.Equal(t, KubernetesAlias(clusterName, "Canary", "default", "my-canary"), access[0].ConfigExternalID.ExternalID)
+
+	expectedRoleAlias := KubernetesAlias(clusterName, "ClusterRole", "", "canary-admin")
+	assert.Equal(t, []string{expectedRoleAlias}, access[0].ExternalRoleAliases)
+	assert.NotEmpty(t, access[0].ID, "access ID should be set")
 }
 
 // Helper types and functions