Skip to content
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.

Sign up for GitHub

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

Adding BypassPullRequestActorIDs to branch protection #1030

Merged
merged 1 commit into from
Mar 11, 2022
Merged

Adding BypassPullRequestActorIDs to branch protection #1030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions github/resource_github_branch_protection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ func resourceGithubBranchProtection() *schema.Resource {
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
PROTECTION_PULL_REQUESTS_BYPASSERS: {
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
},
},
},
Expand Down Expand Up @@ -147,6 +152,7 @@ func resourceGithubBranchProtectionCreate(d *schema.ResourceData, meta interface
input := githubv4.CreateBranchProtectionRuleInput{
AllowsDeletions: githubv4.NewBoolean(githubv4.Boolean(data.AllowsDeletions)),
AllowsForcePushes: githubv4.NewBoolean(githubv4.Boolean(data.AllowsForcePushes)),
BypassPullRequestActorIDs: githubv4NewIDSlice(githubv4IDSliceEmpty(data.BypassPullRequestActorIDs)),
DismissesStaleReviews: githubv4.NewBoolean(githubv4.Boolean(data.DismissesStaleReviews)),
IsAdminEnforced: githubv4.NewBoolean(githubv4.Boolean(data.IsAdminEnforced)),
Pattern: githubv4.String(data.Pattern),
Expand Down Expand Up @@ -275,6 +281,7 @@ func resourceGithubBranchProtectionUpdate(d *schema.ResourceData, meta interface
BranchProtectionRuleID: d.Id(),
AllowsDeletions: githubv4.NewBoolean(githubv4.Boolean(data.AllowsDeletions)),
AllowsForcePushes: githubv4.NewBoolean(githubv4.Boolean(data.AllowsForcePushes)),
BypassPullRequestActorIDs: githubv4NewIDSlice(githubv4IDSliceEmpty(data.BypassPullRequestActorIDs)),
DismissesStaleReviews: githubv4.NewBoolean(githubv4.Boolean(data.DismissesStaleReviews)),
IsAdminEnforced: githubv4.NewBoolean(githubv4.Boolean(data.IsAdminEnforced)),
Pattern: githubv4.NewString(githubv4.String(data.Pattern)),
Expand Down
115 changes: 115 additions & 0 deletions github/resource_github_branch_protection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,121 @@ func TestAccGithubBranchProtection(t *testing.T) {

})

t.Run("configures non-empty list of pull request bypassers", func(t *testing.T) {

config := fmt.Sprintf(`

resource "github_repository" "test" {
name = "tf-acc-test-%s"
auto_init = true
}

resource "github_branch_protection" "test" {

repository_id = github_repository.test.node_id
pattern = "main"

required_pull_request_reviews {
pull_request_bypassers = [
"1234",
]
}

}

`, randomID)

check := resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(
"github_branch_protection.test", "required_pull_request_reviews.0.dismiss_stale_reviews.#", "1",
),
resource.TestCheckResourceAttr(
"github_branch_protection.test", "required_pull_request_reviews.0.dismiss_stale_reviews.0", "1234",
),
)

testCase := func(t *testing.T, mode string) {
resource.Test(t, resource.TestCase{
PreCheck: func() { skipUnlessMode(t, mode) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: config,
Check: check,
},
},
})
}

t.Run("with an anonymous account", func(t *testing.T) {
t.Skip("anonymous account not supported for this operation")
})

t.Run("with an individual account", func(t *testing.T) {
testCase(t, individual)
})

t.Run("with an organization account", func(t *testing.T) {
testCase(t, organization)
})

})

t.Run("configures empty list of pull request bypassers", func(t *testing.T) {

config := fmt.Sprintf(`

resource "github_repository" "test" {
name = "tf-acc-test-%s"
auto_init = true
}

resource "github_branch_protection" "test" {

repository_id = github_repository.test.node_id
pattern = "main"

required_pull_request_reviews {
pull_request_bypassers = []
}

}

`, randomID)

check := resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(
"github_branch_protection.test", "required_pull_request_reviews.0.dismiss_stale_reviews.#", "0",
),
)

testCase := func(t *testing.T, mode string) {
resource.Test(t, resource.TestCase{
PreCheck: func() { skipUnlessMode(t, mode) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: config,
Check: check,
},
},
})
}

t.Run("with an anonymous account", func(t *testing.T) {
t.Skip("anonymous account not supported for this operation")
})

t.Run("with an individual account", func(t *testing.T) {
testCase(t, individual)
})

t.Run("with an organization account", func(t *testing.T) {
testCase(t, organization)
})

})

}

func importBranchProtectionByRepoName(repo, pattern string) resource.ImportStateIdFunc {
Expand Down
8 changes: 8 additions & 0 deletions github/util_v4.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ func githubv4IDSlice(ss []string) []githubv4.ID {
return vGh4
}

func githubv4IDSliceEmpty(ss []string) []githubv4.ID {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is "Empty" in the name here? It looks like the function is a translation between a string slice and a githubv4.ID slice, and I'm not quite understanding how Empty fits in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember it correctly, I had to add this here so we can set the list of pull_request_bypassers to empty list:

resource "github_branch_protection" "test" {
  ...
  required_pull_request_reviews {
    pull_request_bypassers = []
  }
}

Without this we cannot remove all bypassers if any were set before.

I didn't want to make changes to the githubv4IDSlice for the case some other logic would break so I added this new function. If you feel the function should be called differently, plese suggest a new name and I will amend it.

vGh4 := make([]githubv4.ID, 0)
for _, s := range ss {
vGh4 = append(vGh4, githubv4.ID(s))
}
return vGh4
}

func githubv4NewStringSlice(v []githubv4.String) *[]githubv4.String { return &v }

func githubv4NewIDSlice(v []githubv4.ID) *[]githubv4.ID { return &v }
38 changes: 38 additions & 0 deletions github/util_v4_branch_protection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ type DismissalActorTypes struct {
}
}

type BypassPullRequestActorTypes struct {
Actor struct {
Team Actor `graphql:"... on Team"`
User Actor `graphql:"... on User"`
}
}

type PushActorTypes struct {
Actor struct {
App Actor `graphql:"... on App"`
Expand All @@ -39,6 +46,9 @@ type BranchProtectionRule struct {
ReviewDismissalAllowances struct {
Nodes []DismissalActorTypes
} `graphql:"reviewDismissalAllowances(first: 100)"`
BypassPullRequestAllowances struct {
Nodes []BypassPullRequestActorTypes
} `graphql:"bypassPullRequestAllowances(first: 100)"`
AllowsDeletions githubv4.Boolean
AllowsForcePushes githubv4.Boolean
DismissesStaleReviews githubv4.Boolean
Expand All @@ -62,6 +72,7 @@ type BranchProtectionResourceData struct {
AllowsDeletions bool
AllowsForcePushes bool
BranchProtectionRuleID string
BypassPullRequestActorIDs []string
DismissesStaleReviews bool
IsAdminEnforced bool
Pattern string
Expand Down Expand Up @@ -157,6 +168,16 @@ func branchProtectionResourceData(d *schema.ResourceData, meta interface{}) (Bra
data.RestrictsReviewDismissals = true
}
}
if v, ok := m[PROTECTION_PULL_REQUESTS_BYPASSERS]; ok {
bypassPullRequestActorIDs := make([]string, 0)
vL := v.(*schema.Set).List()
for _, v := range vL {
bypassPullRequestActorIDs = append(bypassPullRequestActorIDs, v.(string))
}
if len(bypassPullRequestActorIDs) > 0 {
data.BypassPullRequestActorIDs = bypassPullRequestActorIDs
}
}
}
}

Expand Down Expand Up @@ -210,6 +231,20 @@ func setDismissalActorIDs(actors []DismissalActorTypes) []string {
return pushActors
}

func setBypassPullRequestActorIDs(actors []BypassPullRequestActorTypes) []string {
bypassActors := make([]string, 0, len(actors))
for _, a := range actors {
if a.Actor.Team != (Actor{}) {
bypassActors = append(bypassActors, a.Actor.Team.ID.(string))
}
if a.Actor.User != (Actor{}) {
bypassActors = append(bypassActors, a.Actor.User.ID.(string))
}
Comment on lines +237 to +242
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind explaining this logic to me a little more?

Copy link
Contributor Author

@jtyr jtyr Feb 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is the same like in the case of setPushActorIDs bellow.

}

return bypassActors
}

func setPushActorIDs(actors []PushActorTypes) []string {
pushActors := make([]string, 0, len(actors))
for _, a := range actors {
Expand All @@ -234,13 +269,16 @@ func setApprovingReviews(protection BranchProtectionRule) interface{} {

dismissalAllowances := protection.ReviewDismissalAllowances.Nodes
dismissalActors := setDismissalActorIDs(dismissalAllowances)
bypassPullRequestAllowances := protection.BypassPullRequestAllowances.Nodes
bypassPullRequestActors := setBypassPullRequestActorIDs(bypassPullRequestAllowances)
approvalReviews := []interface{}{
map[string]interface{}{
PROTECTION_REQUIRED_APPROVING_REVIEW_COUNT: protection.RequiredApprovingReviewCount,
PROTECTION_REQUIRES_CODE_OWNER_REVIEWS: protection.RequiresCodeOwnerReviews,
PROTECTION_DISMISSES_STALE_REVIEWS: protection.DismissesStaleReviews,
PROTECTION_RESTRICTS_REVIEW_DISMISSALS: protection.RestrictsReviewDismissals,
PROTECTION_RESTRICTS_REVIEW_DISMISSERS: dismissalActors,
PROTECTION_PULL_REQUESTS_BYPASSERS: bypassPullRequestActors,
},
}

Expand Down
1 change: 1 addition & 0 deletions github/util_v4_consts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const (
PROTECTION_RESTRICTS_PUSHES = "push_restrictions"
PROTECTION_RESTRICTS_REVIEW_DISMISSALS = "restrict_dismissals"
PROTECTION_RESTRICTS_REVIEW_DISMISSERS = "dismissal_restrictions"
PROTECTION_PULL_REQUESTS_BYPASSERS = "pull_request_bypassers"

REPOSITORY_ID = "repository_id"
)
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/hashicorp/hcl/v2 v2.3.0 // indirect
github.com/hashicorp/terraform-config-inspect v0.0.0-20191212124732-c6ae6269b9d7 // indirect
github.com/hashicorp/terraform-plugin-sdk v1.7.0
github.com/shurcooL/githubv4 v0.0.0-20210725200734-83ba7b4c9228
github.com/shurcooL/githubv4 v0.0.0-20220106005112-0707a5a90543
github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f // indirect
github.com/ulikunitz/xz v0.5.10 // indirect
golang.org/x/crypto v0.0.0-20210817164053-32db794688a5
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,8 @@ github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
github.com/shirou/gopsutil v0.0.0-20190901111213-e4ec7b275ada/go.mod h1:WWnYX4lzhCH5h/3YBfyVA3VbLYjlMZZAQcW9ojMexNc=
github.com/shirou/w32 v0.0.0-20160930032740-bb4de0191aa4/go.mod h1:qsXQc7+bwAM3Q1u/4XEfrquwF8Lw7D7y5cD8CuHnfIc=
github.com/shurcooL/githubv4 v0.0.0-20210725200734-83ba7b4c9228 h1:N5B+JgvM/DVYIxreItPJMM3yWrNO/GB2q4nESrtBisM=
github.com/shurcooL/githubv4 v0.0.0-20210725200734-83ba7b4c9228/go.mod h1:hAF0iLZy4td2EX+/8Tw+4nodhlMrwN3HupfaXj3zkGo=
github.com/shurcooL/githubv4 v0.0.0-20220106005112-0707a5a90543 h1:TLml5yQBxKTGrjQQUt+fMcJNNIUyNH0wDeCVGyaLF+s=
github.com/shurcooL/githubv4 v0.0.0-20220106005112-0707a5a90543/go.mod h1:hAF0iLZy4td2EX+/8Tw+4nodhlMrwN3HupfaXj3zkGo=
github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e h1:MZM7FHLqUHYI0Y/mQAt3d2aYa0SiNms/hFqC9qJYolM=
github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e/go.mod h1:TDJrrUr11Vxrven61rcy3hJMUqaf/CLWYhHNPmT14Lk=
github.com/shurcooL/go-goon v0.0.0-20170922171312-37c2f522c041 h1:llrF3Fs4018ePo4+G/HV/uQUqEI1HMDjCeOf2V6puPc=
Expand Down
Loading