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

This bugfix change set gracefully handles checking for the
instance-level default branch protection setting if the provider token
is not from an administrator user.
This bug was introduced in
#1128 in an
attempt to check the instance-level wide settings - which unfortunately,
and not to my knowledge at the time requires an admin token.

In case the token is not from an admin the project resource won't check
for the settings and by default restore the faulty behavior before #1128
which was to wait for the default branch protection to be created.
However, if this won't happen because it's disabled on the
instance-level the user can set
`skip_wait_for_default_branch_protection = true` which skips waiting for
the default branch protection.

Closes: #1167

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