-
Notifications
You must be signed in to change notification settings - Fork 869
feat: Add support for 'exempt' github_repository_ruleset bypass_mode #2764
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Maybe don't replace the |
Ahh, thanks for catching that @krische. My IDE auto-formatted the markdown upon save and I didn't catch it 😅 |
Required: true, | ||
ValidateFunc: validation.StringInSlice([]string{"always", "pull_request"}, false), | ||
ValidateFunc: validation.StringInSlice([]string{"always", "pull_request", "exempt"}, false), | ||
Description: "When the specified actor can bypass the ruleset. pull_request means that an actor can only bypass rules on pull requests. Can be one of: `always`, `pull_request`.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the description needs to be updated too:
Description: "When the specified actor can bypass the ruleset. pull_request means that an actor can only bypass rules on pull requests. Can be one of: `always`, `pull_request`.", | |
Description: "When the specified actor can bypass the ruleset. pull_request means that an actor can only bypass rules on pull requests. Can be one of: `always`, `pull_request`, `exempt`.", |
@joshhunt are you working on requested changes? Would need this to go forward and happy to help. |
Thanks for the reminder @oikarinen. I fixed it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should follow up with some tests - there doesn't seem to be any coverage for any of the bypass_modes, this new one or otherwise 😬
Add test coverage for all three bypass_modes (always, pull_request, exempt) for both repository and organization rulesets. Repository Ruleset Tests (resource_github_repository_ruleset_test.go): - Add test for all three bypass_modes with Team actors - Add test for updating bypass_mode (always → exempt) - Add test for different actor types with different bypass_modes (Team/always, RepositoryRole/pull_request, OrganizationAdmin/exempt) Organization Ruleset Tests (resource_github_organization_ruleset_test.go): - Fix existing "Creates and updates organization using bypasses" test: * Move bypass_actors from incorrect location (inside rules) to correct location (at ruleset level) * Add missing bypass_mode assertions * Fix incorrect actor_type assertions (was checking for "0", "5" instead of actual actor type strings) * Correct OrganizationAdmin actor_id from 0 to 1 - Add test for all three bypass_modes - Add test for updating bypass_mode All tests now verify: - actor_id (both dynamic Team IDs and static role IDs) - actor_type (Team, RepositoryRole, OrganizationAdmin, DeployKey) - bypass_mode (always, pull_request, exempt) This addresses the review comment from PR integrations#2764 requesting test coverage for bypass_modes, including the newly added "exempt" mode. Fixes: integrations#2764 (review)
Hope this helps #2802 |
Add test coverage for all three bypass_modes (always, pull_request, exempt) for both repository and organization rulesets. Repository Ruleset Tests (resource_github_repository_ruleset_test.go): - Add test for all three bypass_modes with Team actors - Add test for updating bypass_mode (always → exempt) - Add test for different actor types with different bypass_modes (Team/always, RepositoryRole/pull_request, OrganizationAdmin/exempt) Organization Ruleset Tests (resource_github_organization_ruleset_test.go): - Fix existing "Creates and updates organization using bypasses" test: * Move bypass_actors from incorrect location (inside rules) to correct location (at ruleset level) * Add missing bypass_mode assertions * Fix incorrect actor_type assertions (was checking for "0", "5" instead of actual actor type strings) * Correct OrganizationAdmin actor_id from 0 to 1 - Add test for all three bypass_modes - Add test for updating bypass_mode All tests now verify: - actor_id (both dynamic Team IDs and static role IDs) - actor_type (Team, RepositoryRole, OrganizationAdmin, DeployKey) - bypass_mode (always, pull_request, exempt) This addresses the review comment from PR #2764 requesting test coverage for bypass_modes, including the newly added "exempt" mode. Fixes: #2764 (review) Co-authored-by: Nick Floyd <[email protected]>
Resolves #2755
I'm not familiar with terraform provider development, so I'm not sure if there's anything else I'm missing, but this looked right to me.
Before the change?
After the change?
Pull request checklist
Does this introduce a breaking change?