Skip to content

Commit f27db8c

Browse files
committed
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.
1 parent 8cd2568 commit f27db8c

File tree

2 files changed

+58
-32
lines changed

2 files changed

+58
-32
lines changed

gitlab/resource_gitlab_project.go

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -915,83 +915,88 @@ func resourceGitlabProjectDelete(ctx context.Context, d *schema.ResourceData, me
915915
func editOrAddPushRules(ctx context.Context, client *gitlab.Client, projectID string, d *schema.ResourceData) error {
916916
log.Printf("[DEBUG] Editing push rules for project %q", projectID)
917917

918-
editOptions := expandEditProjectPushRuleOptions(d)
919-
_, _, err := client.Projects.EditProjectPushRule(projectID, editOptions, gitlab.WithContext(ctx))
920-
if err == nil {
921-
return nil
922-
}
918+
pushRules, _, err := client.Projects.GetProjectPushRules(d.Id(), gitlab.WithContext(ctx))
919+
// NOTE: push rules id `0` indicates that there haven't been any push rules set.
920+
if err != nil || pushRules.ID == 0 {
921+
if addOptions := expandAddProjectPushRuleOptions(d); (gitlab.AddProjectPushRuleOptions{}) != addOptions {
922+
log.Printf("[DEBUG] Creating new push rules for project %q", projectID)
923+
_, _, err = client.Projects.AddProjectPushRule(projectID, &addOptions, gitlab.WithContext(ctx))
924+
if err != nil {
925+
return err
926+
}
927+
} else {
928+
log.Printf("[DEBUG] Don't create new push rules for defaults for project %q", projectID)
929+
}
923930

924-
var httpErr *gitlab.ErrorResponse
925-
if !errors.As(err, &httpErr) || httpErr.Response.StatusCode != http.StatusNotFound {
926-
return err
931+
return nil
927932
}
928933

929-
// A 404 could mean that the push rules need to be re-created.
930-
931-
log.Printf("[DEBUG] Failed to edit push rules for project %q: %v", projectID, err)
932-
log.Printf("[DEBUG] Creating new push rules for project %q", projectID)
933-
934-
addOptions := expandAddProjectPushRuleOptions(d)
935-
_, _, err = client.Projects.AddProjectPushRule(projectID, addOptions, gitlab.WithContext(ctx))
936-
if err != nil {
937-
return err
934+
editOptions := expandEditProjectPushRuleOptions(d, pushRules)
935+
if (gitlab.EditProjectPushRuleOptions{}) != editOptions {
936+
log.Printf("[DEBUG] Editing existing push rules for project %q", projectID)
937+
_, _, err = client.Projects.EditProjectPushRule(projectID, &editOptions, gitlab.WithContext(ctx))
938+
if err != nil {
939+
return err
940+
}
941+
} else {
942+
log.Printf("[DEBUG] Don't edit existing push rules for defaults for project %q", projectID)
938943
}
939944

940945
return nil
941946
}
942947

943-
func expandEditProjectPushRuleOptions(d *schema.ResourceData) *gitlab.EditProjectPushRuleOptions {
944-
options := &gitlab.EditProjectPushRuleOptions{}
948+
func expandEditProjectPushRuleOptions(d *schema.ResourceData, currentPushRules *gitlab.ProjectPushRules) gitlab.EditProjectPushRuleOptions {
949+
options := gitlab.EditProjectPushRuleOptions{}
945950

946-
if d.HasChange("push_rules.0.author_email_regex") {
951+
if d.Get("push_rules.0.author_email_regex") != currentPushRules.AuthorEmailRegex {
947952
options.AuthorEmailRegex = gitlab.String(d.Get("push_rules.0.author_email_regex").(string))
948953
}
949954

950-
if d.HasChange("push_rules.0.branch_name_regex") {
955+
if d.Get("push_rules.0.branch_name_regex") != currentPushRules.BranchNameRegex {
951956
options.BranchNameRegex = gitlab.String(d.Get("push_rules.0.branch_name_regex").(string))
952957
}
953958

954-
if d.HasChange("push_rules.0.commit_message_regex") {
959+
if d.Get("push_rules.0.commit_message_regex") != currentPushRules.CommitMessageRegex {
955960
options.CommitMessageRegex = gitlab.String(d.Get("push_rules.0.commit_message_regex").(string))
956961
}
957962

958-
if d.HasChange("push_rules.0.commit_message_negative_regex") {
963+
if d.Get("push_rules.0.commit_message_negative_regex") != currentPushRules.CommitMessageNegativeRegex {
959964
options.CommitMessageNegativeRegex = gitlab.String(d.Get("push_rules.0.commit_message_negative_regex").(string))
960965
}
961966

962-
if d.HasChange("push_rules.0.file_name_regex") {
967+
if d.Get("push_rules.0.file_name_regex") != currentPushRules.FileNameRegex {
963968
options.FileNameRegex = gitlab.String(d.Get("push_rules.0.file_name_regex").(string))
964969
}
965970

966-
if d.HasChange("push_rules.0.commit_committer_check") {
971+
if d.Get("push_rules.0.commit_committer_check") != currentPushRules.CommitCommitterCheck {
967972
options.CommitCommitterCheck = gitlab.Bool(d.Get("push_rules.0.commit_committer_check").(bool))
968973
}
969974

970-
if d.HasChange("push_rules.0.deny_delete_tag") {
975+
if d.Get("push_rules.0.deny_delete_tag") != currentPushRules.DenyDeleteTag {
971976
options.DenyDeleteTag = gitlab.Bool(d.Get("push_rules.0.deny_delete_tag").(bool))
972977
}
973978

974-
if d.HasChange("push_rules.0.member_check") {
979+
if d.Get("push_rules.0.member_check") != currentPushRules.MemberCheck {
975980
options.MemberCheck = gitlab.Bool(d.Get("push_rules.0.member_check").(bool))
976981
}
977982

978-
if d.HasChange("push_rules.0.prevent_secrets") {
983+
if d.Get("push_rules.0.prevent_secrets") != currentPushRules.PreventSecrets {
979984
options.PreventSecrets = gitlab.Bool(d.Get("push_rules.0.prevent_secrets").(bool))
980985
}
981986

982-
if d.HasChange("push_rules.0.reject_unsigned_commits") {
987+
if d.Get("push_rules.0.reject_unsigned_commits") != currentPushRules.RejectUnsignedCommits {
983988
options.RejectUnsignedCommits = gitlab.Bool(d.Get("push_rules.0.reject_unsigned_commits").(bool))
984989
}
985990

986-
if d.HasChange("push_rules.0.max_file_size") {
991+
if d.Get("push_rules.0.max_file_size") != currentPushRules.MaxFileSize {
987992
options.MaxFileSize = gitlab.Int(d.Get("push_rules.0.max_file_size").(int))
988993
}
989994

990995
return options
991996
}
992997

993-
func expandAddProjectPushRuleOptions(d *schema.ResourceData) *gitlab.AddProjectPushRuleOptions {
994-
options := &gitlab.AddProjectPushRuleOptions{}
998+
func expandAddProjectPushRuleOptions(d *schema.ResourceData) gitlab.AddProjectPushRuleOptions {
999+
options := gitlab.AddProjectPushRuleOptions{}
9951000

9961001
if v, ok := d.GetOk("push_rules.0.author_email_regex"); ok {
9971002
options.AuthorEmailRegex = gitlab.String(v.(string))

gitlab/resource_gitlab_project_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,27 @@ func TestAccGitlabProject_archiveOnDestroy(t *testing.T) {
398398
})
399399
}
400400

401+
func TestAccGitlabProject_setSinglePushRuleToDefault(t *testing.T) {
402+
rInt := acctest.RandInt()
403+
404+
resource.Test(t, resource.TestCase{
405+
PreCheck: func() { testAccPreCheck(t) },
406+
Providers: testAccProviders,
407+
CheckDestroy: testAccCheckGitlabProjectDestroy,
408+
Steps: []resource.TestStep{
409+
{
410+
SkipFunc: isRunningInCE,
411+
Config: testAccGitlabProjectConfigPushRules(rInt, `
412+
member_check = false
413+
`),
414+
Check: testAccCheckGitlabProjectPushRules("gitlab_project.foo", &gitlab.ProjectPushRules{
415+
MemberCheck: false,
416+
}),
417+
},
418+
},
419+
})
420+
}
421+
401422
func TestAccGitlabProject_IssueMergeRequestTemplates(t *testing.T) {
402423
var project gitlab.Project
403424
rInt := acctest.RandInt()

0 commit comments

Comments
 (0)