Fix to provision/deprovision to repos #51
Conversation
WalkthroughThe changes in Changes
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/connector/repo.go (1)
Line range hint
332-335: Update logging statements for consistency.The logging statements still use
DisplayNamewhile the code has been updated to useId.Resource. For consistency and easier troubleshooting, the logs should use the same identifier.l.Info("User Membership has been revoked.", - zap.String("UserName", principal.DisplayName), + zap.String("UserName", principal.Id.Resource), zap.String("ProjectKey", projectKey), zap.String("RepositorySlug", repoSlug), ) l.Info("Group Membership has been revoked.", - zap.String("GroupName", principal.DisplayName), + zap.String("GroupName", principal.Id.Resource), zap.String("ProjectKey", projectKey), zap.String("RepositorySlug", repoSlug), )Also applies to: 360-363
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/connector/repo.go(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
pkg/connector/repo.go (3)
207-207: LGTM! Critical bugfix for user/group identification.The change from
DisplayNametoId.Resourceis correct and addresses the core issue. This ensures that the actual Bitbucket username/groupname is used for permission management instead of the display name, which could differ and cause permission issues.Also applies to: 242-242
327-327: LGTM! Core changes for permission revocation.The use of
Id.Resourcefor permission checks and revocation is correct and consistent with the Grant method changes.Also applies to: 344-344, 355-355
Line range hint
207-363: Verify consistent usage of Id.Resource across the codebase.Let's ensure there are no other instances where DisplayName is used for permission management.
✅ Verification successful
DisplayName usage is correctly limited to display purposes only
The verification confirms that Id.Resource is consistently used for all permission management operations across the codebase, while DisplayName is only used for entitlement metadata and logging. The changes are safe and maintain consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for potential instances where DisplayName might be used for permissions # Look for DisplayName usage in permission-related code echo "Searching for DisplayName usage in permission-related code..." rg -l 'DisplayName.*\b(permission|Permission|PERMISSION)\b' --type go # Look for specific permission update/revoke method calls echo "Searching for permission management method calls..." ast-grep --pattern 'UpdateUserRepositoryPermission($$$)' --lang go ast-grep --pattern 'UpdateGroupRepositoryPermission($$$)' --lang go ast-grep --pattern 'RevokeUserRepositoryPermission($$$)' --lang go ast-grep --pattern 'RevokeGroupRepositoryPermission($$$)' --lang goLength of output: 524
Script:
#!/bin/bash # Show the actual lines containing DisplayName in permission context echo "=== DisplayName usage in permission context ===" rg 'DisplayName.*\b(permission|Permission|PERMISSION)\b' --type go -C 2 # Search for other permission-related methods echo -e "\n=== Other permission-related methods ===" rg '(Update|Revoke|Remove).*Permission' --type go -C 2 # Look for permission-related interfaces and structs echo -e "\n=== Permission-related types ===" ast-grep --pattern 'type $_ interface { $$$ Permission $$$ }' --lang go ast-grep --pattern 'type $_ struct { $$$ Permission $$$ }' --lang goLength of output: 11689
Pull Request
To help us get this pull request reviewed and merged quickly, please be sure to include the following items:
PR Type
What kind of change does this PR introduce?
Bugfix - Updated Grant and Revoke functions for repo.go to use username instead of display name
Backward Compatibility
Is this change backward compatible with the most recently released version? Does it introduce changes which might change the user experience in any way? Does it alter the Connector in any way?
Useful links:
Issue Linking
What's new?
Summary by CodeRabbit