-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: Add private repo workflows permission API support #3679
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
base: master
Are you sure you want to change the base?
feat: Add private repo workflows permission API support #3679
Conversation
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.
Pull Request Overview
This pull request adds API support for managing private repository fork PR workflow permissions across repositories, organizations, and enterprises. It implements 6 new endpoints for getting and setting workflow permissions that control how fork pull requests can run workflows on private repositories.
- Adds
WorkflowsPermissions
struct to represent fork PR workflow settings - Implements repository-level API methods for getting and setting private repo fork PR workflow settings
- Implements organization-level and enterprise-level API methods for the same functionality
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
github/actions_workflows.go | Adds WorkflowsPermissions struct with four boolean fields for controlling fork PR workflow behavior |
github/repos_actions_permissions.go | Implements repository-level GET and PUT methods for private repo fork PR workflow settings |
github/actions_permissions_orgs.go | Implements organization-level GET and PUT methods for private repo fork PR workflow settings |
github/actions_permissions_enterprise.go | Implements enterprise-level GET and PUT methods for private repo fork PR workflow settings |
github/repos_actions_permissions_test.go | Adds comprehensive tests for the repository-level API methods |
github/actions_permissions_orgs_test.go | Adds comprehensive tests for the organization-level API methods |
github/actions_permissions_enterprise_test.go | Adds comprehensive tests for the enterprise-level API methods |
github/github-accessors.go | Adds getter methods for the WorkflowsPermissions struct fields |
github/github-accessors_test.go | Adds tests for the getter methods |
github/github-stringify_test.go | Adds test for the String() method of WorkflowsPermissions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3679 +/- ##
==========================================
+ Coverage 91.07% 91.10% +0.03%
==========================================
Files 186 186
Lines 16563 16625 +62
==========================================
+ Hits 15085 15147 +62
Misses 1293 1293
Partials 185 185 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Thank you, @zyfy29!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
@alexandear or @stevehipwell - might you have time for a code review? Thank you!
// GitHub API docs: https://docs.github.com/enterprise-cloud@latest/rest/actions/permissions#set-private-repo-fork-pr-workflow-settings-for-an-enterprise | ||
// | ||
//meta:operation PUT /enterprises/{enterprise}/actions/permissions/fork-pr-workflows-private-repos | ||
func (s *ActionsService) EditPrivateRepoForkPRWorkflowSettingsInEnterprise(ctx context.Context, enterprise string, permissions WorkflowsPermissions) (*Response, error) { |
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.
According to the doc, the body parameter run_workflows_from_fork_pull_requests
is required. So, we can't reuse WorkflowsPermissions
here because RunWorkflowsFromForkPullRequests
has a pointer type *bool
.
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.
Now I use a mixture of pointer and non-pointer fileds to address that. Thanks for your review!
github/actions_workflows.go
Outdated
RunWorkflowsFromForkPullRequests *bool `json:"run_workflows_from_fork_pull_requests,omitempty"` | ||
SendWriteTokensToWorkflows *bool `json:"send_write_tokens_to_workflows,omitempty"` | ||
SendSecretsAndVariables *bool `json:"send_secrets_and_variables,omitempty"` | ||
RequireApprovalForForkPrWorkflows *bool `json:"require_approval_for_fork_pr_workflows,omitempty"` |
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.
RequireApprovalForForkPrWorkflows *bool `json:"require_approval_for_fork_pr_workflows,omitempty"` | |
RequireApprovalForForkPRWorkflows *bool `json:"require_approval_for_fork_pr_workflows,omitempty"` | |
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.
CC @gmlewis
// GitHub API docs: https://docs.github.com/enterprise-cloud@latest/rest/actions/permissions#set-private-repo-fork-pr-workflow-settings-for-an-enterprise | ||
// | ||
//meta:operation PUT /enterprises/{enterprise}/actions/permissions/fork-pr-workflows-private-repos | ||
func (s *ActionsService) EditPrivateRepoForkPRWorkflowSettingsInEnterprise(ctx context.Context, enterprise string, permissions *WorkflowsPermissionsOpt) (*Response, error) { |
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.
func (s *ActionsService) EditPrivateRepoForkPRWorkflowSettingsInEnterprise(ctx context.Context, enterprise string, permissions *WorkflowsPermissionsOpt) (*Response, error) { | |
func (s *ActionsService) UpdatePrivateRepoForkPRWorkflowSettingsInEnterprise(ctx context.Context, enterprise string, permissions *WorkflowsPermissionsOpt) (*Response, error) { |
I think update makes more sense than edit both here and in the wider context of the package API.
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 you are right, @stevehipwell, thank you.
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.
That's OK, I'll rename 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.
Or may I open another pr to do that? For the permissons APIs above, methods have been named GetXXX
and EditXXX
where both GET and PUT methods are used for the same path. I think we better rename it all at once to keep consistency.
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'm fine either way.
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.
OK, I renamed the methods implemented in this pr for now.
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.
LGTM
}) | ||
} | ||
|
||
func TestActionsService_EditPrivateRepoForkPRWorkflowSettingsInOrganization(t *testing.T) { |
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.
func TestActionsService_EditPrivateRepoForkPRWorkflowSettingsInOrganization(t *testing.T) { | |
func TestActionsService_UpdatePrivateRepoForkPRWorkflowSettingsInOrganization(t *testing.T) { |
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.
Oh, I fix it. Thank you!
}) | ||
} | ||
|
||
func TestActionsService_EditPrivateRepoForkPRWorkflowSettingsInEnterprise(t *testing.T) { |
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.
func TestActionsService_EditPrivateRepoForkPRWorkflowSettingsInEnterprise(t *testing.T) { | |
func TestActionsService_UpdatePrivateRepoForkPRWorkflowSettingsInEnterprise(t *testing.T) { |
3rd Pull Request for #3660
Add support for the "Allowing workflows on fork pull requests in private repositories" APIs, covering 6 endpoints.