resource/gitlab_project: correctly handle push rules add and edit

This is a possible approach to fix #836. Even though, I am not sure if that is
really that elegant of a solution ...

The root problem here is that the SDK v2 doesn't support to check if an
attribute value was set in config, state or the plan. During creation of
push rules, we don't have a way to distinguish between the default and
if the user has set the default explicitly. This makes the behavior
working for all cases (AFAIK) (incl. changing the instance default push
rules, no automated test possible yet).

This will be possbile in the new terraform provider framework.

Author: timofurrer

gitlab/resource_gitlab_project.go

@@ -915,129 +915,141 @@ func resourceGitlabProjectDelete(ctx context.Context, d *schema.ResourceData, me
 func editOrAddPushRules(ctx context.Context, client *gitlab.Client, projectID string, d *schema.ResourceData) error {
 	log.Printf("[DEBUG] Editing push rules for project %q", projectID)
 
-	editOptions := expandEditProjectPushRuleOptions(d)
-	_, _, err := client.Projects.EditProjectPushRule(projectID, editOptions, gitlab.WithContext(ctx))
-	if err == nil {
-		return nil
-	}
+	pushRules, _, err := client.Projects.GetProjectPushRules(d.Id(), gitlab.WithContext(ctx))
+	if is404(err) || pushRules.ID == 0 {
+		if addOptions, needToAdd := expandAddProjectPushRuleOptions(d); needToAdd {
+			log.Printf("[DEBUG] Creating new push rules for project %q", projectID)
+			_, _, err = client.Projects.AddProjectPushRule(projectID, addOptions, gitlab.WithContext(ctx))
+			if err != nil {
+				return err
+			}
+		} else {
+			log.Printf("[DEBUG] Don't create new push rules for defaults for project %q", projectID)
+		}
 
-	var httpErr *gitlab.ErrorResponse
-	if !errors.As(err, &httpErr) || httpErr.Response.StatusCode != http.StatusNotFound {
-		return err
+		return nil
 	}
 
-	// A 404 could mean that the push rules need to be re-created.
-
-	log.Printf("[DEBUG] Failed to edit push rules for project %q: %v", projectID, err)
-	log.Printf("[DEBUG] Creating new push rules for project %q", projectID)
-
-	addOptions := expandAddProjectPushRuleOptions(d)
-	_, _, err = client.Projects.AddProjectPushRule(projectID, addOptions, gitlab.WithContext(ctx))
-	if err != nil {
-		return err
+	editOptions := expandEditProjectPushRuleOptions(d, pushRules)
+	log.Printf("[DEBUG] Editing existing push rules for project %q", projectID)
+	_, _, err = client.Projects.EditProjectPushRule(projectID, editOptions, gitlab.WithContext(ctx))
+	if err == nil {
+		return nil
 	}
 
 	return nil
 }
 
-func expandEditProjectPushRuleOptions(d *schema.ResourceData) *gitlab.EditProjectPushRuleOptions {
+func expandEditProjectPushRuleOptions(d *schema.ResourceData, currentPushRules *gitlab.ProjectPushRules) *gitlab.EditProjectPushRuleOptions {
 	options := &gitlab.EditProjectPushRuleOptions{}
 
-	if d.HasChange("push_rules.0.author_email_regex") {
+	if d.HasChange("push_rules.0.author_email_regex") || d.Get("push_rules.0.author_email_regex") != currentPushRules.AuthorEmailRegex {
 		options.AuthorEmailRegex = gitlab.String(d.Get("push_rules.0.author_email_regex").(string))
 	}
 
-	if d.HasChange("push_rules.0.branch_name_regex") {
+	if d.HasChange("push_rules.0.branch_name_regex") || d.Get("push_rules.0.branch_name_regex") != currentPushRules.BranchNameRegex {
 		options.BranchNameRegex = gitlab.String(d.Get("push_rules.0.branch_name_regex").(string))
 	}
 
-	if d.HasChange("push_rules.0.commit_message_regex") {
+	if d.HasChange("push_rules.0.commit_message_regex") || d.Get("push_rules.0.commit_message_regex") != currentPushRules.CommitMessageRegex {
 		options.CommitMessageRegex = gitlab.String(d.Get("push_rules.0.commit_message_regex").(string))
 	}
 
-	if d.HasChange("push_rules.0.commit_message_negative_regex") {
+	if d.HasChange("push_rules.0.commit_message_negative_regex") || d.Get("push_rules.0.commit_message_negative_regex") != currentPushRules.CommitMessageNegativeRegex {
 		options.CommitMessageNegativeRegex = gitlab.String(d.Get("push_rules.0.commit_message_negative_regex").(string))
 	}
 
-	if d.HasChange("push_rules.0.file_name_regex") {
+	if d.HasChange("push_rules.0.file_name_regex") || d.Get("push_rules.0.file_name_regex") != currentPushRules.FileNameRegex {
 		options.FileNameRegex = gitlab.String(d.Get("push_rules.0.file_name_regex").(string))
 	}
 
-	if d.HasChange("push_rules.0.commit_committer_check") {
+	if d.HasChange("push_rules.0.commit_committer_check") || d.Get("push_rules.0.commit_committer_check") != currentPushRules.CommitCommitterCheck {
 		options.CommitCommitterCheck = gitlab.Bool(d.Get("push_rules.0.commit_committer_check").(bool))
 	}
 
-	if d.HasChange("push_rules.0.deny_delete_tag") {
+	if d.HasChange("push_rules.0.deny_delete_tag") || d.Get("push_rules.0.deny_delete_tag") != currentPushRules.DenyDeleteTag {
 		options.DenyDeleteTag = gitlab.Bool(d.Get("push_rules.0.deny_delete_tag").(bool))
 	}
 
-	if d.HasChange("push_rules.0.member_check") {
+	if d.HasChange("push_rules.0.member_check") || d.Get("push_rules.0.member_check") != currentPushRules.MemberCheck {
 		options.MemberCheck = gitlab.Bool(d.Get("push_rules.0.member_check").(bool))
 	}
 
-	if d.HasChange("push_rules.0.prevent_secrets") {
+	if d.HasChange("push_rules.0.prevent_secrets") || d.Get("push_rules.0.prevent_secrets") != currentPushRules.PreventSecrets {
 		options.PreventSecrets = gitlab.Bool(d.Get("push_rules.0.prevent_secrets").(bool))
 	}
 
-	if d.HasChange("push_rules.0.reject_unsigned_commits") {
+	if d.HasChange("push_rules.0.reject_unsigned_commits") || d.Get("push_rules.0.reject_unsigned_commits") != currentPushRules.RejectUnsignedCommits {
 		options.RejectUnsignedCommits = gitlab.Bool(d.Get("push_rules.0.reject_unsigned_commits").(bool))
 	}
 
-	if d.HasChange("push_rules.0.max_file_size") {
+	if d.HasChange("push_rules.0.max_file_size") || d.Get("push_rules.0.max_file_size") != currentPushRules.MaxFileSize {
 		options.MaxFileSize = gitlab.Int(d.Get("push_rules.0.max_file_size").(int))
 	}
 
 	return options
 }
 
-func expandAddProjectPushRuleOptions(d *schema.ResourceData) *gitlab.AddProjectPushRuleOptions {
+func expandAddProjectPushRuleOptions(d *schema.ResourceData) (*gitlab.AddProjectPushRuleOptions, bool) {
 	options := &gitlab.AddProjectPushRuleOptions{}
+	needToAdd := false
 
 	if v, ok := d.GetOk("push_rules.0.author_email_regex"); ok {
 		options.AuthorEmailRegex = gitlab.String(v.(string))
+		needToAdd = true
 	}
 
 	if v, ok := d.GetOk("push_rules.0.branch_name_regex"); ok {
 		options.BranchNameRegex = gitlab.String(v.(string))
+		needToAdd = true
 	}
 
 	if v, ok := d.GetOk("push_rules.0.commit_message_regex"); ok {
 		options.CommitMessageRegex = gitlab.String(v.(string))
+		needToAdd = true
 	}
 
 	if v, ok := d.GetOk("push_rules.0.commit_message_negative_regex"); ok {
 		options.CommitMessageNegativeRegex = gitlab.String(v.(string))
+		needToAdd = true
 	}
 
 	if v, ok := d.GetOk("push_rules.0.file_name_regex"); ok {
 		options.FileNameRegex = gitlab.String(v.(string))
+		needToAdd = true
 	}
 
 	if v, ok := d.GetOk("push_rules.0.commit_committer_check"); ok {
 		options.CommitCommitterCheck = gitlab.Bool(v.(bool))
+		needToAdd = true
 	}
 
 	if v, ok := d.GetOk("push_rules.0.deny_delete_tag"); ok {
 		options.DenyDeleteTag = gitlab.Bool(v.(bool))
+		needToAdd = true
 	}
 
 	if v, ok := d.GetOk("push_rules.0.member_check"); ok {
 		options.MemberCheck = gitlab.Bool(v.(bool))
+		needToAdd = true
 	}
 
 	if v, ok := d.GetOk("push_rules.0.prevent_secrets"); ok {
 		options.PreventSecrets = gitlab.Bool(v.(bool))
+		needToAdd = true
 	}
 
 	if v, ok := d.GetOk("push_rules.0.reject_unsigned_commits"); ok {
 		options.RejectUnsignedCommits = gitlab.Bool(v.(bool))
+		needToAdd = true
 	}
 
 	if v, ok := d.GetOk("push_rules.0.max_file_size"); ok {
 		options.MaxFileSize = gitlab.Int(v.(int))
+		needToAdd = true
 	}
 
-	return options
+	return options, needToAdd
 }
 
 func flattenProjectPushRules(pushRules *gitlab.ProjectPushRules) (values []map[string]interface{}) {

gitlab/resource_gitlab_project_test.go

@@ -398,6 +398,27 @@ func TestAccGitlabProject_archiveOnDestroy(t *testing.T) {
 	})
 }
 
+func TestAccGitlabProject_setSinglePushRuleToDefault(t *testing.T) {
+	rInt := acctest.RandInt()
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckGitlabProjectDestroy,
+		Steps: []resource.TestStep{
+			{
+				SkipFunc: isRunningInCE,
+				Config: testAccGitlabProjectConfigPushRules(rInt, `
+member_check = false
+`),
+				Check: testAccCheckGitlabProjectPushRules("gitlab_project.foo", &gitlab.ProjectPushRules{
+					MemberCheck: false,
+				}),
+			},
+		},
+	})
+}
+
 func TestAccGitlabProject_IssueMergeRequestTemplates(t *testing.T) {
 	var project gitlab.Project
 	rInt := acctest.RandInt()