Add code_owner_approval_required argument to gitlab_branch_protection resource

armsnyder · armsnyder · commit ab0e4e1f4c9e · 2020-07-16T16:52:53.000-07:00
diff --git a/gitlab/resource_gitlab_branch_protection.go b/gitlab/resource_gitlab_branch_protection.go
@@ -40,6 +40,10 @@ func resourceGitlabBranchProtection() *schema.Resource {
 				Required:     true,
 				ForceNew:     true,
 			},
+			"code_owner_approval_required": {
+				Type:     schema.TypeBool,
+				Optional: true,
+			},
 		},
 	}
 }
@@ -50,11 +54,13 @@ func resourceGitlabBranchProtectionCreate(d *schema.ResourceData, meta interface
 	branch := gitlab.String(d.Get("branch").(string))
 	mergeAccessLevel := accessLevelID[d.Get("merge_access_level").(string)]
 	pushAccessLevel := accessLevelID[d.Get("push_access_level").(string)]
+	codeOwnerApprovalRequired := getOptionalBool(d, "code_owner_approval_required")
 
 	options := &gitlab.ProtectRepositoryBranchesOptions{
-		Name:             branch,
-		MergeAccessLevel: &mergeAccessLevel,
-		PushAccessLevel:  &pushAccessLevel,
+		Name:                      branch,
+		MergeAccessLevel:          &mergeAccessLevel,
+		PushAccessLevel:           &pushAccessLevel,
+		CodeOwnerApprovalRequired: codeOwnerApprovalRequired,
 	}
 
 	log.Printf("[DEBUG] create gitlab branch protection on %v for project %s", options.Name, project)
@@ -98,6 +104,7 @@ func resourceGitlabBranchProtectionRead(d *schema.ResourceData, meta interface{}
 	d.Set("branch", pb.Name)
 	d.Set("merge_access_level", accessLevel[pb.MergeAccessLevels[0].AccessLevel])
 	d.Set("push_access_level", accessLevel[pb.PushAccessLevels[0].AccessLevel])
+	d.Set("code_owner_approval_required", pb.CodeOwnerApprovalRequired)
 
 	d.SetId(buildTwoPartID(&project, &pb.Name))
 
diff --git a/gitlab/resource_gitlab_branch_protection_test.go b/gitlab/resource_gitlab_branch_protection_test.go
@@ -2,6 +2,7 @@ package gitlab
 
 import (
 	"fmt"
+	"strconv"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/helper/acctest"
@@ -59,6 +60,66 @@ func TestAccGitlabBranchProtection_basic(t *testing.T) {
 					}),
 				),
 			},
+			// Update the the Branch Protection code owner approval setting
+			{
+				SkipFunc: isRunningInCE,
+				Config:   testAccGitlabBranchProtectionUpdateConfigCodeOwnerTrue(rInt),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.BranchProtect", &pb),
+					testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.BranchProtect", &pb),
+					testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
+						Name:                      fmt.Sprintf("BranchProtect-%d", rInt),
+						PushAccessLevel:           accessLevel[gitlab.DeveloperPermissions],
+						MergeAccessLevel:          accessLevel[gitlab.DeveloperPermissions],
+						CodeOwnerApprovalRequired: true,
+					}),
+				),
+			},
+			// Update the Branch Protection to get back to initial settings.
+			// Since code_owner_approval_required is an optional setting, it does not revert to its original setting.
+			// This test ensures that the addition of this optional value is backward compatible and does not overwrite existing settings in GitLab when it is unset.
+			{
+				SkipFunc: isRunningInCE,
+				Config:   testAccGitlabBranchProtectionConfig(rInt),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.BranchProtect", &pb),
+					testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.BranchProtect", &pb),
+					testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
+						Name:                      fmt.Sprintf("BranchProtect-%d", rInt),
+						PushAccessLevel:           accessLevel[gitlab.DeveloperPermissions],
+						MergeAccessLevel:          accessLevel[gitlab.DeveloperPermissions],
+						CodeOwnerApprovalRequired: true,
+					}),
+				),
+			},
+			// Update the the Branch Protection to explicitly disable code owner approval, to truly reset state back to initial settings.
+			{
+				SkipFunc: isRunningInCE,
+				Config:   testAccGitlabBranchProtectionUpdateConfigCodeOwnerFalse(rInt),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.BranchProtect", &pb),
+					testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.BranchProtect", &pb),
+					testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
+						Name:             fmt.Sprintf("BranchProtect-%d", rInt),
+						PushAccessLevel:  accessLevel[gitlab.DeveloperPermissions],
+						MergeAccessLevel: accessLevel[gitlab.DeveloperPermissions],
+					}),
+				),
+			},
+			// Update the Branch Protection to get back to initial settings.
+			{
+				SkipFunc: isRunningInCE,
+				Config:   testAccGitlabBranchProtectionConfig(rInt),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.BranchProtect", &pb),
+					testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.BranchProtect", &pb),
+					testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
+						Name:             fmt.Sprintf("BranchProtect-%d", rInt),
+						PushAccessLevel:  accessLevel[gitlab.DeveloperPermissions],
+						MergeAccessLevel: accessLevel[gitlab.DeveloperPermissions],
+					}),
+				),
+			},
 		},
 	})
 }
@@ -78,6 +139,10 @@ func testAccCheckGitlabBranchProtectionPersistsInStateCorrectly(n string, pb *gi
 			return fmt.Errorf("push access level not persisted in state correctly")
 		}
 
+		if rs.Primary.Attributes["code_owner_approval_required"] != strconv.FormatBool(pb.CodeOwnerApprovalRequired) {
+			return fmt.Errorf("code_owner_approval_required not persisted in state correctly")
+		}
+
 		return nil
 	}
 }
@@ -110,9 +175,10 @@ func testAccCheckGitlabBranchProtectionExists(n string, pb *gitlab.ProtectedBran
 }
 
 type testAccGitlabBranchProtectionExpectedAttributes struct {
-	Name             string
-	PushAccessLevel  string
-	MergeAccessLevel string
+	Name                      string
+	PushAccessLevel           string
+	MergeAccessLevel          string
+	CodeOwnerApprovalRequired bool
 }
 
 func testAccCheckGitlabBranchProtectionAttributes(pb *gitlab.ProtectedBranch, want *testAccGitlabBranchProtectionExpectedAttributes) resource.TestCheckFunc {
@@ -129,6 +195,10 @@ func testAccCheckGitlabBranchProtectionAttributes(pb *gitlab.ProtectedBranch, wa
 			return fmt.Errorf("got Merge access levels %q; want %q", pb.MergeAccessLevels[0].AccessLevel, accessLevelID[want.MergeAccessLevel])
 		}
 
+		if pb.CodeOwnerApprovalRequired != want.CodeOwnerApprovalRequired {
+			return fmt.Errorf("got code_owner_approval_required %v; want %v", pb.CodeOwnerApprovalRequired, want.CodeOwnerApprovalRequired)
+		}
+
 		return nil
 	}
 }
@@ -196,3 +266,45 @@ resource "gitlab_branch_protection" "BranchProtect" {
 }
 	`, rInt, rInt)
 }
+
+func testAccGitlabBranchProtectionUpdateConfigCodeOwnerTrue(rInt int) string {
+	return fmt.Sprintf(`
+resource "gitlab_project" "foo" {
+  name = "foo-%d"
+  description = "Terraform acceptance tests"
+
+  # So that acceptance tests can be run in a gitlab organization
+  # with no billing
+  visibility_level = "public"
+}
+
+resource "gitlab_branch_protection" "BranchProtect" {
+  project = "${gitlab_project.foo.id}"
+  branch = "BranchProtect-%d"
+  push_access_level = "developer"
+  merge_access_level = "developer"
+  code_owner_approval_required = true
+}
+	`, rInt, rInt)
+}
+
+func testAccGitlabBranchProtectionUpdateConfigCodeOwnerFalse(rInt int) string {
+	return fmt.Sprintf(`
+resource "gitlab_project" "foo" {
+  name = "foo-%d"
+  description = "Terraform acceptance tests"
+
+  # So that acceptance tests can be run in a gitlab organization
+  # with no billing
+  visibility_level = "public"
+}
+
+resource "gitlab_branch_protection" "BranchProtect" {
+  project = "${gitlab_project.foo.id}"
+  branch = "BranchProtect-%d"
+  push_access_level = "developer"
+  merge_access_level = "developer"
+  code_owner_approval_required = false
+}
+	`, rInt, rInt)
+}
diff --git a/gitlab/util.go b/gitlab/util.go
@@ -190,3 +190,12 @@ func stringSetToStringSlice(stringSet *schema.Set) *[]string {
 	}
 	return &ret
 }
+
+func getOptionalBool(d *schema.ResourceData, key string) *bool {
+	var result *bool
+	if value, isSet := d.GetOk(key); isSet {
+		valueBool := value.(bool)
+		result = &valueBool
+	}
+	return result
+}
diff --git a/website/docs/r/branch_protection.html.markdown b/website/docs/r/branch_protection.html.markdown
@@ -25,10 +25,12 @@ resource "gitlab_branch_protection" "BranchProtect" {
 
 The following arguments are supported:
 
-* `project` - (Required) The id of the project.
+* `project` - (Required, string) The id of the project.
 
-* `branch` - (Required) Name of the branch.
+* `branch` - (Required, string) Name of the branch.
 
-* `push_access_level` - (Required) One of five levels of access to the project.
+* `push_access_level` - (Required, string) One of five levels of access to the project.
 
-* `merge_access_level` - (Required) One of five levels of access to the project.
+* `merge_access_level` - (Required, string) One of five levels of access to the project.
+
+* `code_owner_approval_required` (Optional, bool) Can be set to true to enable code owner approval requirement. Requires GitLab Silver/Premium.

Original file line number	Diff line number	Diff line change
`@@ -190,3 +190,12 @@ func stringSetToStringSlice(stringSet schema.Set) []string {`
`190`	`190`	`}`
`191`	`191`	`return &ret`
`192`	`192`	`}`
	`193`	`+`
	`194`	`+func getOptionalBool(d schema.ResourceData, key string) bool {`
	`195`	`+ var result *bool`
	`196`	`+ if value, isSet := d.GetOk(key); isSet {`
	`197`	`+ valueBool := value.(bool)`
	`198`	`+ result = &valueBool`
	`199`	`+ }`
	`200`	`+ return result`
	`201`	`+}`