[BB-1886] Fix role lookups for privilege grants

[johnallers]

Fixes various issues:

• Fixes role lookup when generating grants to roles for privileges.
• Fixes error when looking up non-existent role: "No access"

[coderabbitai]

Walkthrough

The PR modifies role grant handling in the connector by introducing a "No access" role skip condition in collaborator grants and refactoring role metadata retrieval in the role resolver to use role traits (id, name, profile) from the resource instead of directly accessing resource properties.

Changes

Cohort / File(s)	Summary
Role Grant Handling pkg/connector/collaborator.go, pkg/connector/role.go	collaborator.go adds a noAccessRoleName constant and skips processing roles matching "No access" during grant construction. role.go refactors role metadata retrieval to use RoleTrait for profile/id/name validation; switches role lookups to use roleId; updates error paths for missing role attributes and not-found scenarios; preserves existing privilege extraction behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Session rework for containerization #34 — Directly modifies the same code paths in collaboratorRoleGrants and role grant resolution logic
• [BB-1788] Add containerization support implementing ResourceSyncerV2 and SessionStore cache #29 — Modifies role and collaborator grant construction, overlapping with the role/grants handling refactored here

Suggested reviewers

• JavierCarnelli-ConductorOne
• agustin-conductor
• luisina-santos
• mateovespConductor

Poem

🐰 A role without access, we simply skip past,
With traits now in hand, the lookups hold fast,
No more DisplayName, just ids that align,
The grants flow more cleanly, one hop at a time! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing role lookups for privilege grants, which aligns with the modifications to role.go and collaborator.go that improve role identification and lookup logic.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

pkg/connector/role.go (1)

126-141: LGTM! Minor optimization opportunity.

The role trait extraction logic is correct and provides proper error handling. Consider caching the profile.AsMap() result to avoid calling it twice:

🔎 Optional: cache AsMap() result

 	profile := roleTrait.GetProfile()
 	if profile == nil {
 		return nil, nil, fmt.Errorf("role profile not found on resource %s", resource.Id.Resource)
 	}
-	roleId, ok := profile.AsMap()["id"].(string)
+	profileMap := profile.AsMap()
+	roleId, ok := profileMap["id"].(string)
 	if !ok {
 		return nil, nil, fmt.Errorf("role id not found on resource %s", resource.Id.Resource)
 	}
-	roleName, ok := profile.AsMap()["name"].(string)
+	roleName, ok := profileMap["name"].(string)
 	if !ok {
 		return nil, nil, fmt.Errorf("role name not found on resource %s", resource.Id.Resource)
 	}

pkg/connector/collaborator.go (1)

208-209: Correctly skips "No access" role during grant construction.

The placement after the base role check is appropriate since "No access" is not a base role, and skipping it prevents attempting to create grants for a role that doesn't exist as a grantable resource.

For consistency, consider defining noAccessRoleName in the workato package alongside the base roles, which would centralize all role-related constants:

#!/bin/bash
# Check if there are other role name constants defined in the workato package
rg -n "RoleName|role.*=" pkg/connector/workato/roles.go

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57e39a8 and fa0fa88.

📒 Files selected for processing (2)

• pkg/connector/collaborator.go
• pkg/connector/role.go

🧰 Additional context used

🧬 Code graph analysis (1)

pkg/connector/role.go (1)

pkg/connector/workato/roles.go (2)

• IsBaseRole (18-25)
• GetBaseRole (27-38)

🔇 Additional comments (3)

pkg/connector/role.go (2)

144-145: Correct alignment with base role lookup functions.

Using roleName from the role trait profile aligns correctly with workato.IsBaseRole() and workato.GetBaseRole(), which compare against role.RoleName as shown in the relevant code snippets.

---

180-184: Good fix for role lookup by ID.

The change to use roleId extracted from the role trait profile (instead of resource.Id.Resource) correctly resolves the lookup issue for custom roles. The enhanced error message including both display name and roleId improves debuggability.

pkg/connector/collaborator.go (1)

23-26: Good use of constant to avoid magic string.