Merge pull request #1169 from timofurrer/bugfix/1167

resource/gitlab_project: Fix admin token requirement to check default branch protection

Author: timofurrer

CHANGELOG.md

@@ -1,31 +1,39 @@
+## 3.16.1 (2022-07-11)
+
+This release was tested against GitLab 14.10, 15.0 and 15.1 for both CE and EE.
+
+BUG FIXES:
+
+* resource/gitlab_project: Fix admin token requirement to check default branch protection ([#1169](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1169))
+
 ## 3.16.0 (2022-07-07)
 
 This release was tested against GitLab 14.10, 15.0 and 15.1 for both CE and EE.
 
 FEATURES:
 
-* **New Data Source:** `gitlab_current_user` ([#1118])
-* **New Data Source:** `gitlab_release_link` ([#1131])
-* **New Data Source:** `gitlab_release_links` ([#1131])
-* **New Resource:** `gitlab_release_link` ([#1131])
-* **New Resource:** `gitlab_cluster_agent_token` ([#1147])
+* **New Data Source:** `gitlab_current_user` ([#1118](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1118))
+* **New Data Source:** `gitlab_release_link` ([#1131](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1131))
+* **New Data Source:** `gitlab_release_links` ([#1131](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1131))
+* **New Resource:** `gitlab_release_link` ([#1131](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1131))
+* **New Resource:** `gitlab_cluster_agent_token` ([#1147](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1147))
 
 IMPROVEMENTS:
 
-* resource/gitlab_project_protected_environment: Add `required_approval_count` attribute ([#1097])
-* resource/gitlab_project_access_token: Add `owner` as possible value to `access_level` ([#1145])
-* resource/gitlab_project_membership: Add `owner` as possible value to `access_level` ([#1145])
-* resource/gitlab_project_share_group: Add `owner` as possible value to `access_level` ([#1145])
-* resource/gitlab_project: Add `ci_default_git_depth` attribute ([#1146])
-* datasource/gitlab_project: Add `ci_default_git_depth` attribute ([#1146])
-* datasource/gitlab_projects: Add `ci_default_git_depth` attribute ([#1146])
+* resource/gitlab_project_protected_environment: Add `required_approval_count` attribute ([#1097](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1097))
+* resource/gitlab_project_access_token: Add `owner` as possible value to `access_level` ([#1145](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1145))
+* resource/gitlab_project_membership: Add `owner` as possible value to `access_level` ([#1145](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1145))
+* resource/gitlab_project_share_group: Add `owner` as possible value to `access_level` ([#1145](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1145))
+* resource/gitlab_project: Add `ci_default_git_depth` attribute ([#1146](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1146))
+* datasource/gitlab_project: Add `ci_default_git_depth` attribute ([#1146](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1146))
+* datasource/gitlab_projects: Add `ci_default_git_depth` attribute ([#1146](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1146))
 
 BUG FIXES:
 
-* resource/gitlab_project: Fix project creation when default branch protection is disabled on instance-level (#[1128])
-* resource/gitlab_project: Fix a case where a change to a project in terraform can never apply when certain fields are modified ([#1158])
-* resource/gitlab_project: Fix passing `false` to API for explicitly set optional attributes ([#1152])
-* resource/gitlab_group: Fix passing false to API for explicitly set optional attributes ([#1152])
+* resource/gitlab_project: Fix project creation when default branch protection is disabled on instance-level ([#1128](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1128))
+* resource/gitlab_project: Fix a case where a change to a project in terraform can never apply when certain fields are modified ([#1158](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1158))
+* resource/gitlab_project: Fix passing `false` to API for explicitly set optional attributes ([#1152](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1152))
+* resource/gitlab_group: Fix passing false to API for explicitly set optional attributes ([#1152](https://github.com/gitlabhq/terraform-provider-gitlab/pull/1152))
 
 ## 3.15.1 (2022-06-08)
 

docs/resources/personal_access_token.md

@@ -12,7 +12,7 @@ description: |-
 
 The `gitlab_personal_access_token` resource allows to manage the lifecycle of a personal access token for a specified user.
 
--> This resource requires administration privileges. 
+-> This resource requires administration privileges.
 
 **Upstream API**: [GitLab REST API docs](https://docs.gitlab.com/ee/api/personal_access_tokens.html)
 

docs/resources/project.md

@@ -136,6 +136,10 @@ resource "gitlab_project" "peters_repo" {
 - `resolve_outdated_diff_discussions` (Boolean) Automatically resolve merge request diffs discussions on lines changed with a push.
 - `security_and_compliance_access_level` (String) Set the security and compliance access level. Valid values are `disabled`, `private`, `enabled`.
 - `shared_runners_enabled` (Boolean) Enable shared runners for this project.
+- `skip_wait_for_default_branch_protection` (Boolean) If `true`, the default behavior to wait for the default branch protection to be created is skipped.
+This is necessary if the current user is not an admin and the default branch protection is disabled on an instance-level.
+There is currently no known way to determine if the default branch protection is disabled on an instance-level for non-admin users.
+This attribute is only used during resource creation, thus changes are suppressed and the attribute cannot be imported.
 - `snippets_access_level` (String) Set the snippets access level. Valid values are `disabled`, `private`, `enabled`.
 - `snippets_enabled` (Boolean) Enable snippets for the project.
 - `squash_commit_template` (String) Template used to create squash commit message in merge requests. (Introduced in GitLab 14.6.)

internal/provider/resource_gitlab_personal_access_tokens.go

@@ -29,7 +29,7 @@ var _ = registerResource("gitlab_personal_access_token", func() *schema.Resource
 	return &schema.Resource{
 		Description: `The ` + "`gitlab_personal_access_token`" + ` resource allows to manage the lifecycle of a personal access token for a specified user.
 
--> This resource requires administration privileges. 
+-> This resource requires administration privileges.
 
 **Upstream API**: [GitLab REST API docs](https://docs.gitlab.com/ee/api/personal_access_tokens.html)`,
 
@@ -97,7 +97,7 @@ var _ = registerResource("gitlab_personal_access_token", func() *schema.Resource
 func resourceGitlabPersonalAccessTokenCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	client := meta.(*gitlab.Client)
 
-	currentUserAdmin, err := isCurrentUserAdmin(client)
+	currentUserAdmin, err := isCurrentUserAdmin(ctx, client)
 	if err != nil {
 		return diag.Errorf("[ERROR] cannot query the user API for current user: %v", err)
 	}

internal/provider/resource_gitlab_project.go

@@ -671,7 +671,20 @@ branch using a ` + "`DELETE`" + ` request. Then define the desired branch protec
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},
-		Schema: resourceGitLabProjectSchema,
+		Schema: constructSchema(resourceGitLabProjectSchema, map[string]*schema.Schema{
+			"skip_wait_for_default_branch_protection": {
+				Description: `If ` + "`true`" + `, the default behavior to wait for the default branch protection to be created is skipped.
+This is necessary if the current user is not an admin and the default branch protection is disabled on an instance-level.
+There is currently no known way to determine if the default branch protection is disabled on an instance-level for non-admin users.
+This attribute is only used during resource creation, thus changes are suppressed and the attribute cannot be imported.
+`,
+				Type:     schema.TypeBool,
+				Optional: true,
+				DiffSuppressFunc: func(string, string, string, *schema.ResourceData) bool {
+					return true
+				},
+			},
+		}),
 		CustomizeDiff: customdiff.All(
 			customdiff.ComputedIf("path_with_namespace", namespaceOrPathChanged),
 			customdiff.ComputedIf("ssh_url_to_repo", namespaceOrPathChanged),
@@ -1106,41 +1119,48 @@ func resourceGitlabProjectCreate(ctx context.Context, d *schema.ResourceData, me
 		}
 	}
 
-	// If the project is assigned to a group namespace and the group has *default branch protection*
-	// disabled (`default_branch_protection = 0`) then we don't have to wait for one.
-	waitForDefaultBranchProtection, err := expectDefaultBranchProtection(ctx, client, project)
-	if err != nil {
-		return diag.Errorf("Failed to discover if branch protection is enabled by default or not for project %d: %+v", project.ID, err)
+	// nolint:staticcheck // SA1019 ignore deprecated GetOkExists
+	// lintignore: XR001 // TODO: replace with alternative for GetOkExists
+	if v, ok := d.GetOkExists("skip_wait_for_default_branch_protection"); ok {
+		d.Set("skip_wait_for_default_branch_protection", v.(bool))
 	}
+	if !d.Get("skip_wait_for_default_branch_protection").(bool) {
+		// If the project is assigned to a group namespace and the group has *default branch protection*
+		// disabled (`default_branch_protection = 0`) then we don't have to wait for one.
+		waitForDefaultBranchProtection, err := expectDefaultBranchProtection(ctx, client, project)
+		if err != nil {
+			return diag.Errorf("Failed to discover if branch protection is enabled by default or not for project %d: %+v", project.ID, err)
+		}
 
-	if waitForDefaultBranchProtection {
-		// Branch protection for a newly created branch is an async action, so use WaitForState to ensure it's protected
-		// before we continue. Note this check should only be required when there is a custom default branch set
-		// See issue 800: https://github.com/gitlabhq/terraform-provider-gitlab/issues/800
-		stateConf := &resource.StateChangeConf{
-			Pending: []string{"false"},
-			Target:  []string{"true"},
-			Timeout: 2 * time.Minute, //The async action usually completes very quickly, within seconds. Don't wait too long.
-			Refresh: func() (interface{}, string, error) {
-				branch, _, err := client.Branches.GetBranch(project.ID, project.DefaultBranch, gitlab.WithContext(ctx))
-				if err != nil {
-					if is404(err) {
-						// When we hit a 404 here, it means the default branch wasn't created at all as part of the project
-						// this will happen when "default_branch" isn't set, or "initialize_with_readme" is set to false.
-						// We don't need to wait anymore, so return "true" to exist the wait loop.
-						return branch, "true", nil
+		if waitForDefaultBranchProtection {
+			// Branch protection for a newly created branch is an async action, so use WaitForState to ensure it's protected
+			// before we continue. Note this check should only be required when there is a custom default branch set
+			// See issue 800: https://github.com/gitlabhq/terraform-provider-gitlab/issues/800
+			stateConf := &resource.StateChangeConf{
+				Pending: []string{"false"},
+				Target:  []string{"true"},
+				Timeout: 2 * time.Minute, //The async action usually completes very quickly, within seconds. Don't wait too long.
+				Refresh: func() (interface{}, string, error) {
+					branch, _, err := client.Branches.GetBranch(project.ID, project.DefaultBranch, gitlab.WithContext(ctx))
+					if err != nil {
+						if is404(err) {
+							// When we hit a 404 here, it means the default branch wasn't created at all as part of the project
+							// this will happen when "default_branch" isn't set, or "initialize_with_readme" is set to false.
+							// We don't need to wait anymore, so return "true" to exist the wait loop.
+							return branch, "true", nil
+						}
+
+						//This is legit error, return the error.
+						return nil, "", err
 					}
 
-					//This is legit error, return the error.
-					return nil, "", err
-				}
-
-				return branch, strconv.FormatBool(branch.Protected), nil
-			},
-		}
+					return branch, strconv.FormatBool(branch.Protected), nil
+				},
+			}
 
-		if _, err := stateConf.WaitForStateContext(ctx); err != nil {
-			return diag.Errorf("error while waiting for branch %s to reach 'protected' status, %s", project.DefaultBranch, err)
+			if _, err := stateConf.WaitForStateContext(ctx); err != nil {
+				return diag.Errorf("error while waiting for branch %s to reach 'protected' status, %s", project.DefaultBranch, err)
+			}
 		}
 	}
 
@@ -1824,11 +1844,22 @@ func expectDefaultBranchProtection(ctx context.Context, client *gitlab.Client, p
 		return group.DefaultBranchProtection != 0, nil
 	}
 
-	// // If the project is not part of a group it may have default branch protection disabled because of the instance-wide application settings
-	settings, _, err := client.Settings.GetSettings(nil, gitlab.WithContext(ctx))
+	isAdmin, err := isCurrentUserAdmin(ctx, client)
 	if err != nil {
-		return false, err
+		return false, fmt.Errorf("failed to check if user is admin to verify is default branch protection is enabled on instance-level: %w", err)
+	}
+
+	if isAdmin {
+		// If the project is not part of a group it may have default branch protection disabled because of the instance-wide application settings
+		settings, _, err := client.Settings.GetSettings(nil, gitlab.WithContext(ctx))
+		if err != nil {
+			return false, err
+		}
+
+		return settings.DefaultBranchProtection != 0, nil
 	}
 
-	return settings.DefaultBranchProtection != 0, nil
+	// NOTE: for the lack of a better solution (at least for now), we assume that the default branch protection is NOT disabled on instance-level.
+	//       To override this behavior it's best to set `skip_wait_for_default_branch_protection = true` in the resource config.
+	return true, nil
 }

internal/provider/resource_gitlab_project_test.go

@@ -977,6 +977,67 @@ func TestAccGitlabProject_InstanceBranchProtectionDisabled(t *testing.T) {
 				ImportStateVerify:       true,
 				ImportStateVerifyIgnore: []string{"initialize_with_readme"},
 			},
+			// Force a destroy for the project so that it can be recreated as the same resource
+			{
+				Config: ` `, // requires a space for empty config
+			},
+			// With `skip_wait_for_default_branch_protection` enabled
+			{
+				Config: fmt.Sprintf(`
+					resource "gitlab_project" "foo" {
+						name                   = "foo-%d-custom-default-branch"
+						description            = "Terraform acceptance tests"
+						visibility_level       = "public"
+						initialize_with_readme = true
+
+						skip_wait_for_default_branch_protection = true
+					}
+				`, rInt),
+			},
+			// Verify Import
+			{
+				ResourceName:            "gitlab_project.foo",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"initialize_with_readme", "skip_wait_for_default_branch_protection"},
+			},
+			// Force a destroy for the project so that it can be recreated as the same resource
+			{
+				Config: ` `, // requires a space for empty config
+			},
+			{
+				Config: fmt.Sprintf(`
+					resource "gitlab_project" "foo" {
+						name                   = "foo-%d-custom-default-branch"
+						description            = "Terraform acceptance tests"
+						visibility_level       = "public"
+						initialize_with_readme = true
+
+						skip_wait_for_default_branch_protection = false
+					}
+				`, rInt),
+			},
+			// Check if plan is empty after changing `skip_wait_for_default_branch_protection` attribute
+			{
+				Config: fmt.Sprintf(`
+					resource "gitlab_project" "foo" {
+						name                   = "foo-%d-custom-default-branch"
+						description            = "Terraform acceptance tests"
+						visibility_level       = "public"
+						initialize_with_readme = true
+
+						skip_wait_for_default_branch_protection = true
+					}
+				`, rInt),
+				PlanOnly: true,
+			},
+			// Verify Import
+			{
+				ResourceName:            "gitlab_project.foo",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"initialize_with_readme", "skip_wait_for_default_branch_protection"},
+			},
 		},
 	})
 }

internal/provider/util.go

@@ -312,17 +312,13 @@ func is404(err error) bool {
 	return false
 }
 
-func isCurrentUserAdmin(client *gitlab.Client) (bool, error) {
-	currentUser, _, err := client.Users.CurrentUser()
+func isCurrentUserAdmin(ctx context.Context, client *gitlab.Client) (bool, error) {
+	currentUser, _, err := client.Users.CurrentUser(gitlab.WithContext(ctx))
 	if err != nil {
 		return false, err
 	}
 
-	if currentUser.IsAdmin {
-		return true, nil
-	} else {
-		return false, nil
-	}
+	return currentUser.IsAdmin, nil
 }
 
 // ISO 8601 date format