Skip to content

Commit ef97dc9

Browse files
armsnyderarmsnyder
authored
Merge pull request #359 from armsnyder/310-codeowners-approval
Add code_owner_approval_required argument to gitlab_branch_protection resource
2 parents f586dc3 + 0fdce63 commit ef97dc9

File tree

3 files changed

+201
-19
lines changed

3 files changed

+201
-19
lines changed

‎gitlab/resource_gitlab_branch_protection.go‎

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package gitlab
22

33
import (
4+
"errors"
5+
"fmt"
46
"log"
57

68
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
@@ -16,6 +18,7 @@ func resourceGitlabBranchProtection() *schema.Resource {
1618
return &schema.Resource{
1719
Create: resourceGitlabBranchProtectionCreate,
1820
Read: resourceGitlabBranchProtectionRead,
21+
Update: resourceGitlabBranchProtectionUpdate,
1922
Delete: resourceGitlabBranchProtectionDelete,
2023
Schema: map[string]*schema.Schema{
2124
"project": {
@@ -40,6 +43,11 @@ func resourceGitlabBranchProtection() *schema.Resource {
4043
Required: true,
4144
ForceNew: true,
4245
},
46+
"code_owner_approval_required": {
47+
Type: schema.TypeBool,
48+
Optional: true,
49+
Default: false,
50+
},
4351
},
4452
}
4553
}
@@ -50,11 +58,13 @@ func resourceGitlabBranchProtectionCreate(d *schema.ResourceData, meta interface
5058
branch := gitlab.String(d.Get("branch").(string))
5159
mergeAccessLevel := accessLevelID[d.Get("merge_access_level").(string)]
5260
pushAccessLevel := accessLevelID[d.Get("push_access_level").(string)]
61+
codeOwnerApprovalRequired := d.Get("code_owner_approval_required").(bool)
5362

5463
options := &gitlab.ProtectRepositoryBranchesOptions{
55-
Name: branch,
56-
MergeAccessLevel: &mergeAccessLevel,
57-
PushAccessLevel: &pushAccessLevel,
64+
Name: branch,
65+
MergeAccessLevel: &mergeAccessLevel,
66+
PushAccessLevel: &pushAccessLevel,
67+
CodeOwnerApprovalRequired: &codeOwnerApprovalRequired,
5868
}
5969

6070
log.Printf("[DEBUG] create gitlab branch protection on %v for project %s", options.Name, project)
@@ -75,7 +85,17 @@ func resourceGitlabBranchProtectionCreate(d *schema.ResourceData, meta interface
7585

7686
d.SetId(buildTwoPartID(&project, &bp.Name))
7787

78-
return resourceGitlabBranchProtectionRead(d, meta)
88+
if err := resourceGitlabBranchProtectionRead(d, meta); err != nil {
89+
return err
90+
}
91+
92+
// If the GitLab tier does not support the code owner approval feature, the resulting plan will be inconsistent.
93+
// We return an error because otherwise Terraform would report this inconsistency as a "bug in the provider" to the user.
94+
if codeOwnerApprovalRequired && !d.Get("code_owner_approval_required").(bool) {
95+
return errors.New("feature unavailable: code owner approvals")
96+
}
97+
98+
return nil
7999
}
80100

81101
func resourceGitlabBranchProtectionRead(d *schema.ResourceData, meta interface{}) error {
@@ -98,12 +118,41 @@ func resourceGitlabBranchProtectionRead(d *schema.ResourceData, meta interface{}
98118
d.Set("branch", pb.Name)
99119
d.Set("merge_access_level", accessLevel[pb.MergeAccessLevels[0].AccessLevel])
100120
d.Set("push_access_level", accessLevel[pb.PushAccessLevels[0].AccessLevel])
121+
d.Set("code_owner_approval_required", pb.CodeOwnerApprovalRequired)
101122

102123
d.SetId(buildTwoPartID(&project, &pb.Name))
103124

104125
return nil
105126
}
106127

128+
func resourceGitlabBranchProtectionUpdate(d *schema.ResourceData, meta interface{}) error {
129+
// NOTE: At the time of writing, the only value that does not force re-creation is code_owner_approval_required,
130+
// so therefore that is the only update that needs to be handled.
131+
132+
client := meta.(*gitlab.Client)
133+
project := d.Get("project").(string)
134+
branch := d.Get("branch").(string)
135+
codeOwnerApprovalRequired := d.Get("code_owner_approval_required").(bool)
136+
137+
log.Printf("[DEBUG] update gitlab branch protection for project %s, branch %s", project, branch)
138+
139+
options := &gitlab.RequireCodeOwnerApprovalsOptions{
140+
CodeOwnerApprovalRequired: &codeOwnerApprovalRequired,
141+
}
142+
143+
if _, err := client.ProtectedBranches.RequireCodeOwnerApprovals(project, branch, options); err != nil {
144+
// The user might be running a version of GitLab that does not support this feature.
145+
// We enhance the generic 404 error with a more informative message.
146+
if errResponse, ok := err.(*gitlab.ErrorResponse); ok && errResponse.Response.StatusCode == 404 {
147+
return fmt.Errorf("feature unavailable: code owner approvals: %w", err)
148+
}
149+
150+
return err
151+
}
152+
153+
return resourceGitlabBranchProtectionRead(d, meta)
154+
}
155+
107156
func resourceGitlabBranchProtectionDelete(d *schema.ResourceData, meta interface{}) error {
108157
client := meta.(*gitlab.Client)
109158
project := d.Get("project").(string)
@@ -119,7 +168,7 @@ func projectAndBranchFromID(id string) (string, string, error) {
119168
project, branch, err := parseTwoPartID(id)
120169

121170
if err != nil {
122-
log.Printf("[WARN] cannot get group member id from input: %v", id)
171+
log.Printf("[WARN] cannot get branch protection id from input: %v", id)
123172
}
124173
return project, branch, err
125174
}

‎gitlab/resource_gitlab_branch_protection_test.go‎

Lines changed: 144 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package gitlab
22

33
import (
44
"fmt"
5+
"regexp"
6+
"strconv"
57
"testing"
68

79
"github.com/hashicorp/terraform-plugin-sdk/helper/acctest"
@@ -24,8 +26,8 @@ func TestAccGitlabBranchProtection_basic(t *testing.T) {
2426
{
2527
Config: testAccGitlabBranchProtectionConfig(rInt),
2628
Check: resource.ComposeTestCheckFunc(
27-
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.BranchProtect", &pb),
28-
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.BranchProtect", &pb),
29+
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.branch_protect", &pb),
30+
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.branch_protect", &pb),
2931
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
3032
Name: fmt.Sprintf("BranchProtect-%d", rInt),
3133
PushAccessLevel: accessLevel[gitlab.DeveloperPermissions],
@@ -37,8 +39,8 @@ func TestAccGitlabBranchProtection_basic(t *testing.T) {
3739
{
3840
Config: testAccGitlabBranchProtectionUpdateConfig(rInt),
3941
Check: resource.ComposeTestCheckFunc(
40-
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.BranchProtect", &pb),
41-
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.BranchProtect", &pb),
42+
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.branch_protect", &pb),
43+
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.branch_protect", &pb),
4244
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
4345
Name: fmt.Sprintf("BranchProtect-%d", rInt),
4446
PushAccessLevel: accessLevel[gitlab.MasterPermissions],
@@ -50,8 +52,107 @@ func TestAccGitlabBranchProtection_basic(t *testing.T) {
5052
{
5153
Config: testAccGitlabBranchProtectionConfig(rInt),
5254
Check: resource.ComposeTestCheckFunc(
53-
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.BranchProtect", &pb),
54-
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.BranchProtect", &pb),
55+
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.branch_protect", &pb),
56+
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.branch_protect", &pb),
57+
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
58+
Name: fmt.Sprintf("BranchProtect-%d", rInt),
59+
PushAccessLevel: accessLevel[gitlab.DeveloperPermissions],
60+
MergeAccessLevel: accessLevel[gitlab.DeveloperPermissions],
61+
}),
62+
),
63+
},
64+
// Update the the Branch Protection code owner approval setting
65+
{
66+
SkipFunc: isRunningInCE,
67+
Config: testAccGitlabBranchProtectionUpdateConfigCodeOwnerTrue(rInt),
68+
Check: resource.ComposeTestCheckFunc(
69+
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.branch_protect", &pb),
70+
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.branch_protect", &pb),
71+
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
72+
Name: fmt.Sprintf("BranchProtect-%d", rInt),
73+
PushAccessLevel: accessLevel[gitlab.DeveloperPermissions],
74+
MergeAccessLevel: accessLevel[gitlab.DeveloperPermissions],
75+
CodeOwnerApprovalRequired: true,
76+
}),
77+
),
78+
},
79+
// Attempting to update code owner approval setting on CE should fail safely and with an informative error message
80+
{
81+
SkipFunc: isRunningInEE,
82+
Config: testAccGitlabBranchProtectionUpdateConfigCodeOwnerTrue(rInt),
83+
ExpectError: regexp.MustCompile("feature unavailable: code owner approvals"),
84+
Check: resource.ComposeTestCheckFunc(
85+
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.branch_protect", &pb),
86+
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.branch_protect", &pb),
87+
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
88+
Name: fmt.Sprintf("BranchProtect-%d", rInt),
89+
PushAccessLevel: accessLevel[gitlab.DeveloperPermissions],
90+
MergeAccessLevel: accessLevel[gitlab.DeveloperPermissions],
91+
}),
92+
),
93+
},
94+
// Update the Branch Protection to get back to initial settings
95+
{
96+
Config: testAccGitlabBranchProtectionConfig(rInt),
97+
Check: resource.ComposeTestCheckFunc(
98+
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.branch_protect", &pb),
99+
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.branch_protect", &pb),
100+
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
101+
Name: fmt.Sprintf("BranchProtect-%d", rInt),
102+
PushAccessLevel: accessLevel[gitlab.DeveloperPermissions],
103+
MergeAccessLevel: accessLevel[gitlab.DeveloperPermissions],
104+
}),
105+
),
106+
},
107+
},
108+
})
109+
}
110+
111+
func TestAccGitlabBranchProtection_createWithCodeOwnerApproval(t *testing.T) {
112+
var pb gitlab.ProtectedBranch
113+
rInt := acctest.RandInt()
114+
115+
resource.Test(t, resource.TestCase{
116+
PreCheck: func() { testAccPreCheck(t) },
117+
Providers: testAccProviders,
118+
CheckDestroy: testAccCheckGitlabBranchProtectionDestroy,
119+
Steps: []resource.TestStep{
120+
// Create a project and Branch Protection with code owner approval enabled
121+
{
122+
SkipFunc: isRunningInCE,
123+
Config: testAccGitlabBranchProtectionUpdateConfigCodeOwnerTrue(rInt),
124+
Check: resource.ComposeTestCheckFunc(
125+
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.branch_protect", &pb),
126+
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.branch_protect", &pb),
127+
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
128+
Name: fmt.Sprintf("BranchProtect-%d", rInt),
129+
PushAccessLevel: accessLevel[gitlab.DeveloperPermissions],
130+
MergeAccessLevel: accessLevel[gitlab.DeveloperPermissions],
131+
CodeOwnerApprovalRequired: true,
132+
}),
133+
),
134+
},
135+
// Attempting to update code owner approval setting on CE should fail safely and with an informative error message
136+
{
137+
SkipFunc: isRunningInEE,
138+
Config: testAccGitlabBranchProtectionUpdateConfigCodeOwnerTrue(rInt),
139+
ExpectError: regexp.MustCompile("feature unavailable: code owner approvals"),
140+
Check: resource.ComposeTestCheckFunc(
141+
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.branch_protect", &pb),
142+
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.branch_protect", &pb),
143+
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
144+
Name: fmt.Sprintf("BranchProtect-%d", rInt),
145+
PushAccessLevel: accessLevel[gitlab.DeveloperPermissions],
146+
MergeAccessLevel: accessLevel[gitlab.DeveloperPermissions],
147+
}),
148+
),
149+
},
150+
// Update the Branch Protection to get back to initial settings
151+
{
152+
Config: testAccGitlabBranchProtectionConfig(rInt),
153+
Check: resource.ComposeTestCheckFunc(
154+
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.branch_protect", &pb),
155+
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.branch_protect", &pb),
55156
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
56157
Name: fmt.Sprintf("BranchProtect-%d", rInt),
57158
PushAccessLevel: accessLevel[gitlab.DeveloperPermissions],
@@ -78,6 +179,10 @@ func testAccCheckGitlabBranchProtectionPersistsInStateCorrectly(n string, pb *gi
78179
return fmt.Errorf("push access level not persisted in state correctly")
79180
}
80181

182+
if rs.Primary.Attributes["code_owner_approval_required"] != strconv.FormatBool(pb.CodeOwnerApprovalRequired) {
183+
return fmt.Errorf("code_owner_approval_required not persisted in state correctly")
184+
}
185+
81186
return nil
82187
}
83188
}
@@ -110,9 +215,10 @@ func testAccCheckGitlabBranchProtectionExists(n string, pb *gitlab.ProtectedBran
110215
}
111216

112217
type testAccGitlabBranchProtectionExpectedAttributes struct {
113-
Name string
114-
PushAccessLevel string
115-
MergeAccessLevel string
218+
Name string
219+
PushAccessLevel string
220+
MergeAccessLevel string
221+
CodeOwnerApprovalRequired bool
116222
}
117223

118224
func testAccCheckGitlabBranchProtectionAttributes(pb *gitlab.ProtectedBranch, want *testAccGitlabBranchProtectionExpectedAttributes) resource.TestCheckFunc {
@@ -129,6 +235,10 @@ func testAccCheckGitlabBranchProtectionAttributes(pb *gitlab.ProtectedBranch, wa
129235
return fmt.Errorf("got Merge access levels %q; want %q", pb.MergeAccessLevels[0].AccessLevel, accessLevelID[want.MergeAccessLevel])
130236
}
131237

238+
if pb.CodeOwnerApprovalRequired != want.CodeOwnerApprovalRequired {
239+
return fmt.Errorf("got code_owner_approval_required %v; want %v", pb.CodeOwnerApprovalRequired, want.CodeOwnerApprovalRequired)
240+
}
241+
132242
return nil
133243
}
134244
}
@@ -168,8 +278,8 @@ resource "gitlab_project" "foo" {
168278
visibility_level = "public"
169279
}
170280
171-
resource "gitlab_branch_protection" "BranchProtect" {
172-
project = "${gitlab_project.foo.id}"
281+
resource "gitlab_branch_protection" "branch_protect" {
282+
project = gitlab_project.foo.id
173283
branch = "BranchProtect-%d"
174284
push_access_level = "developer"
175285
merge_access_level = "developer"
@@ -188,11 +298,32 @@ resource "gitlab_project" "foo" {
188298
visibility_level = "public"
189299
}
190300
191-
resource "gitlab_branch_protection" "BranchProtect" {
192-
project = "${gitlab_project.foo.id}"
301+
resource "gitlab_branch_protection" "branch_protect" {
302+
project = gitlab_project.foo.id
193303
branch = "BranchProtect-%d"
194304
push_access_level = "maintainer"
195305
merge_access_level = "maintainer"
196306
}
197307
`, rInt, rInt)
198308
}
309+
310+
func testAccGitlabBranchProtectionUpdateConfigCodeOwnerTrue(rInt int) string {
311+
return fmt.Sprintf(`
312+
resource "gitlab_project" "foo" {
313+
name = "foo-%d"
314+
description = "Terraform acceptance tests"
315+
316+
# So that acceptance tests can be run in a gitlab organization
317+
# with no billing
318+
visibility_level = "public"
319+
}
320+
321+
resource "gitlab_branch_protection" "branch_protect" {
322+
project = gitlab_project.foo.id
323+
branch = "BranchProtect-%d"
324+
push_access_level = "developer"
325+
merge_access_level = "developer"
326+
code_owner_approval_required = true
327+
}
328+
`, rInt, rInt)
329+
}

‎website/docs/r/branch_protection.html.markdown‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,6 @@ The following arguments are supported:
3131

3232
* `push_access_level` - (Required) One of five levels of access to the project.
3333

34-
* `merge_access_level` - (Required) One of five levels of access to the project.
34+
* `merge_access_level` - (Required) One of five levels of access to the project.
35+
36+
* `code_owner_approval_required` (Optional) Bool, defaults to false. Can be set to true to require code owner approval before merging.

0 commit comments

Comments
 (0)