Skip to content

Commit fc9fcb8

Browse files
committed
Change code_owner_approval_required from optional/computed to optional/default
This means that users who have managed this flag out of band in the past will see diffs after upgrading plugin versions, but based on my reading this is preferable to using the computed flag for backward-compatibility.
1 parent 8899eff commit fc9fcb8

File tree

2 files changed

+9
-57
lines changed

2 files changed

+9
-57
lines changed

gitlab/resource_gitlab_branch_protection.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func resourceGitlabBranchProtection() *schema.Resource {
4444
"code_owner_approval_required": {
4545
Type: schema.TypeBool,
4646
Optional: true,
47-
Computed: true,
47+
Default: false,
4848
},
4949
},
5050
}
@@ -132,8 +132,6 @@ func resourceGitlabBranchProtectionUpdate(d *schema.ResourceData, meta interface
132132
return err
133133
}
134134

135-
d.SetId(buildTwoPartID(&project, &branch))
136-
137135
return resourceGitlabBranchProtectionRead(d, meta)
138136
}
139137

@@ -152,7 +150,7 @@ func projectAndBranchFromID(id string) (string, string, error) {
152150
project, branch, err := parseTwoPartID(id)
153151

154152
if err != nil {
155-
log.Printf("[WARN] cannot get group member id from input: %v", id)
153+
log.Printf("[WARN] cannot get branch protection id from input: %v", id)
156154
}
157155
return project, branch, err
158156
}

gitlab/resource_gitlab_branch_protection_test.go

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

33
import (
44
"fmt"
5+
"regexp"
56
"strconv"
67
"testing"
78

@@ -75,41 +76,15 @@ func TestAccGitlabBranchProtection_basic(t *testing.T) {
7576
}),
7677
),
7778
},
78-
// Update the Branch Protection to get back to initial settings.
79-
// Since code_owner_approval_required is an optional setting, it does not revert to its original setting.
80-
// 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.
79+
// Attempting to update code owner approval setting on CE should fail safely
8180
{
82-
SkipFunc: isRunningInCE,
83-
Config: testAccGitlabBranchProtectionConfig(rInt),
84-
Check: resource.ComposeTestCheckFunc(
85-
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.BranchProtect", &pb),
86-
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.BranchProtect", &pb),
87-
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
88-
Name: fmt.Sprintf("BranchProtect-%d", rInt),
89-
PushAccessLevel: accessLevel[gitlab.DeveloperPermissions],
90-
MergeAccessLevel: accessLevel[gitlab.DeveloperPermissions],
91-
CodeOwnerApprovalRequired: true,
92-
}),
93-
),
94-
},
95-
// Update the the Branch Protection to explicitly disable code owner approval, to truly reset state back to initial settings.
96-
{
97-
SkipFunc: isRunningInCE,
98-
Config: testAccGitlabBranchProtectionUpdateConfigCodeOwnerFalse(rInt),
99-
Check: resource.ComposeTestCheckFunc(
100-
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.BranchProtect", &pb),
101-
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.BranchProtect", &pb),
102-
testAccCheckGitlabBranchProtectionAttributes(&pb, &testAccGitlabBranchProtectionExpectedAttributes{
103-
Name: fmt.Sprintf("BranchProtect-%d", rInt),
104-
PushAccessLevel: accessLevel[gitlab.DeveloperPermissions],
105-
MergeAccessLevel: accessLevel[gitlab.DeveloperPermissions],
106-
}),
107-
),
81+
SkipFunc: isRunningInEE,
82+
Config: testAccGitlabBranchProtectionUpdateConfigCodeOwnerTrue(rInt),
83+
ExpectError: regexp.MustCompile("404 Not Found"),
10884
},
109-
// Update the Branch Protection to get back to initial settings.
85+
// Update the Branch Protection to get back to initial settings
11086
{
111-
SkipFunc: isRunningInCE,
112-
Config: testAccGitlabBranchProtectionConfig(rInt),
87+
Config: testAccGitlabBranchProtectionConfig(rInt),
11388
Check: resource.ComposeTestCheckFunc(
11489
testAccCheckGitlabBranchProtectionExists("gitlab_branch_protection.BranchProtect", &pb),
11590
testAccCheckGitlabBranchProtectionPersistsInStateCorrectly("gitlab_branch_protection.BranchProtect", &pb),
@@ -287,24 +262,3 @@ resource "gitlab_branch_protection" "BranchProtect" {
287262
}
288263
`, rInt, rInt)
289264
}
290-
291-
func testAccGitlabBranchProtectionUpdateConfigCodeOwnerFalse(rInt int) string {
292-
return fmt.Sprintf(`
293-
resource "gitlab_project" "foo" {
294-
name = "foo-%d"
295-
description = "Terraform acceptance tests"
296-
297-
# So that acceptance tests can be run in a gitlab organization
298-
# with no billing
299-
visibility_level = "public"
300-
}
301-
302-
resource "gitlab_branch_protection" "BranchProtect" {
303-
project = "${gitlab_project.foo.id}"
304-
branch = "BranchProtect-%d"
305-
push_access_level = "developer"
306-
merge_access_level = "developer"
307-
code_owner_approval_required = false
308-
}
309-
`, rInt, rInt)
310-
}

0 commit comments

Comments
 (0)