From bdcc10cbe3925f6c0627bccf0db229e464b17307 Mon Sep 17 00:00:00 2001 From: Marcelo Boeira Date: Fri, 4 Aug 2023 11:36:30 +0200 Subject: [PATCH] fix(gitlab): Prevent nil pointer dereference when HeadPipeline is empty In this particular example `mr.HeadPipeline.SHA` panics on a nil pointer dereference because HeadPipeline is empty. This seems to be caused by the lack of permission to update the commit status. ```go runtime.gopanic runtime/panic.go:1038 runtime.panicmem runtime/panic.go:221 runtime.sigpanic runtime/signal_unix.go:735 github.com/runatlantis/atlantis/server/events/vcs.(*GitlabClient).PullIsMergeable github.com/runatlantis/atlantis/server/events/vcs/gitlab_client.go:208 github.com/runatlantis/atlantis/server/events/vcs.(*ClientProxy).PullIsMergeable github.com/runatlantis/atlantis/server/events/vcs/proxy.go:72 github.com/runatlantis/atlantis/server/events/vcs.(*pullReqStatusFetcher).FetchPullStatus github.com/runatlantis/atlantis/server/events/vcs/pull_status_fetcher.go:28 github.com/runatlantis/atlantis/server/events.(*ApplyCommandRunner).Run github.com/runatlantis/atlantis/server/events/apply_command_runner.go:105 github.com/runatlantis/atlantis/server/events.(*DefaultCommandRunner).RunCommentCommand github.com/runatlantis/atlantis/server/events/command_runner.go:252 ``` The least invasive solution is to simply use the commit-hash from pull and guess that the pipeline was "skipped" unless the HeadPipeline is there. The outcome is: When mr.HeadPipeline is present: - use the commit hash and status from the HeadPipeline When mr.HeadPipeline is NOT present: - use the commit hash from pull request struct - assume the pipeline was "skipped" In cases where GitLab is configured to require a pipeline to pass, this results on a message saying the MR is not mergeable. More info: - https://github.com/runatlantis/atlantis/issues/1852 --- scripts/e2e-deps.sh | 2 +- server/events/vcs/gitlab_client.go | 12 ++++++-- server/events/vcs/gitlab_client_test.go | 37 ++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/scripts/e2e-deps.sh b/scripts/e2e-deps.sh index b01559bad9..4ac9b9272b 100755 --- a/scripts/e2e-deps.sh +++ b/scripts/e2e-deps.sh @@ -18,7 +18,7 @@ unzip terraform_${TERRAFORM_VERSION}_linux_amd64.zip chmod +x terraform cp terraform /home/circleci/go/bin # Download ngrok to create a tunnel to expose atlantis server -wget https://bin.equinox.io/c/bNyj1mQVY4c/ngrok-v2-stable-linux-amd64.zip -O ngrok-stable-linux-amd64.zip +wget https://bin.equinox.io/c/bNyj1mQVY4c/ngrok-v2-stable-linux-amd64.zip -O ngrok-stable-linux-amd64.zip unzip ngrok-stable-linux-amd64.zip chmod +x ngrok # Download jq diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index cd1a05645c..f96d964fda 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -280,6 +280,15 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest return false, err } + // Prevent nil pointer error when mr.HeadPipeline is empty + // See: https://github.com/runatlantis/atlantis/issues/1852 + commit := pull.HeadCommit + isPipelineSkipped := true + if mr.HeadPipeline != nil { + commit = mr.HeadPipeline.SHA + isPipelineSkipped = mr.HeadPipeline.Status == "skipped" + } + // Get project configuration project, _, err := g.Client.Projects.GetProject(mr.ProjectID, nil) if err != nil { @@ -287,7 +296,7 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest } // Get Commit Statuses - statuses, _, err := g.Client.Commits.GetCommitStatuses(mr.ProjectID, mr.HeadPipeline.SHA, nil) + statuses, _, err := g.Client.Commits.GetCommitStatuses(mr.ProjectID, commit, nil) if err != nil { return false, err } @@ -302,7 +311,6 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest } } - isPipelineSkipped := mr.HeadPipeline.Status == "skipped" allowSkippedPipeline := project.AllowMergeOnSkippedPipeline && isPipelineSkipped ok, err := g.SupportsDetailedMergeStatus() diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index 674015c978..8a5d05add5 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -323,40 +323,69 @@ func TestGitlabClient_PullIsMergeable(t *testing.T) { gitlabVersionUnder15_6 := "15.3.2-ce" gitlabServerVersions := []string{gitlabVersionOver15_6, gitlabVersion15_6, gitlabVersionUnder15_6} vcsStatusName := "atlantis-test" + defaultMR = 1 + noHeadPipelineMR = 2 cases := []struct { statusName string status models.CommitStatus gitlabVersion []string + mrId int expState bool }{ { fmt.Sprintf("%s/apply: resource/default", vcsStatusName), models.FailedCommitStatus, gitlabServerVersions, + defaultMr, true, }, { fmt.Sprintf("%s/apply", vcsStatusName), models.FailedCommitStatus, gitlabServerVersions, + defaultMr, true, }, { fmt.Sprintf("%s/plan: resource/default", vcsStatusName), models.FailedCommitStatus, gitlabServerVersions, + defaultMr, false, }, { fmt.Sprintf("%s/plan", vcsStatusName), models.PendingCommitStatus, gitlabServerVersions, + defaultMr, false, }, { fmt.Sprintf("%s/plan", vcsStatusName), models.SuccessCommitStatus, gitlabServerVersions, + defaultMr, + true, + }, + { + fmt.Sprintf("%s/plan", vcsStatusName), + models.PendingCommitStatus, + gitlabServerVersions, + noHeadPipelineMR, + false, + }, + { + fmt.Sprintf("%s/plan", vcsStatusName), + models.FailedCommitStatus, + gitlabServerVersions, + noHeadPipelineMR, + false, + }, + { + fmt.Sprintf("%s/plan", vcsStatusName), + models.SuccessCommitStatus, + gitlabServerVersions, + noHeadPipelineMR, true, }, } @@ -372,6 +401,9 @@ func TestGitlabClient_PullIsMergeable(t *testing.T) { case "/api/v4/projects/runatlantis%2Fatlantis/merge_requests/1": w.WriteHeader(http.StatusOK) w.Write([]byte(pipelineSuccess)) // nolint: errcheck + case "/api/v4/projects/runatlantis%2Fatlantis/merge_requests/2": + w.WriteHeader(http.StatusOK) + w.Write([]byte(headPipelineNotAvailable)) // nolint: errcheck case "/api/v4/projects/4580910": w.WriteHeader(http.StatusOK) w.Write([]byte(projectSuccess)) // nolint: errcheck @@ -409,11 +441,13 @@ func TestGitlabClient_PullIsMergeable(t *testing.T) { Hostname: "gitlab.com", }, } + mergeable, err := client.PullIsMergeable(repo, models.PullRequest{ - Num: 1, + Num: c.mrId, BaseRepo: repo, HeadCommit: "sha", }, vcsStatusName) + Ok(t, err) Equals(t, c.expState, mergeable) }) @@ -554,3 +588,4 @@ var pipelineSuccess = `{"id": 22461274,"iid": 13,"project_id": 4580910,"title": var projectSuccess = `{"id": 4580910,"description": "","name": "atlantis-example","name_with_namespace": "lkysow / atlantis-example","path": "atlantis-example","path_with_namespace": "lkysow/atlantis-example","created_at": "2018-04-30T13:44:28.367Z","default_branch": "patch-1","tag_list": [],"ssh_url_to_repo": "git@gitlab.com:lkysow/atlantis-example.git","http_url_to_repo": "https://gitlab.com/lkysow/atlantis-example.git","web_url": "https://gitlab.com/lkysow/atlantis-example","readme_url": "https://gitlab.com/lkysow/atlantis-example/-/blob/main/README.md","avatar_url": "https://gitlab.com/uploads/-/system/project/avatar/4580910/avatar.png","forks_count": 0,"star_count": 7,"last_activity_at": "2021-06-29T21:10:43.968Z","namespace": {"id": 1,"name": "lkysow","path": "lkysow","kind": "group","full_path": "lkysow","parent_id": 1,"avatar_url": "/uploads/-/system/group/avatar/1651/platform.png","web_url": "https://gitlab.com/groups/lkysow"},"_links": {"self": "https://gitlab.com/api/v4/projects/4580910","issues": "https://gitlab.com/api/v4/projects/4580910/issues","merge_requests": "https://gitlab.com/api/v4/projects/4580910/merge_requests","repo_branches": "https://gitlab.com/api/v4/projects/4580910/repository/branches","labels": "https://gitlab.com/api/v4/projects/4580910/labels","events": "https://gitlab.com/api/v4/projects/4580910/events","members": "https://gitlab.com/api/v4/projects/4580910/members"},"packages_enabled": false,"empty_repo": false,"archived": false,"visibility": "private","resolve_outdated_diff_discussions": false,"container_registry_enabled": false,"container_expiration_policy": {"cadence": "1d","enabled": false,"keep_n": 10,"older_than": "90d","name_regex": ".*","name_regex_keep": null,"next_run_at": "2021-05-01T13:44:28.397Z"},"issues_enabled": true,"merge_requests_enabled": true,"wiki_enabled": false,"jobs_enabled": true,"snippets_enabled": true,"service_desk_enabled": false,"service_desk_address": null,"can_create_merge_request_in": true,"issues_access_level": "private","repository_access_level": "enabled","merge_requests_access_level": "enabled","forking_access_level": "enabled","wiki_access_level": "disabled","builds_access_level": "enabled","snippets_access_level": "enabled","pages_access_level": "private","operations_access_level": "disabled","analytics_access_level": "enabled","emails_disabled": null,"shared_runners_enabled": true,"lfs_enabled": false,"creator_id": 818,"import_status": "none","import_error": null,"open_issues_count": 0,"runners_token": "1234456","ci_default_git_depth": 50,"ci_forward_deployment_enabled": true,"public_jobs": true,"build_git_strategy": "fetch","build_timeout": 3600,"auto_cancel_pending_pipelines": "enabled","build_coverage_regex": null,"ci_config_path": "","shared_with_groups": [],"only_allow_merge_if_pipeline_succeeds": true,"allow_merge_on_skipped_pipeline": false,"restrict_user_defined_variables": false,"request_access_enabled": true,"only_allow_merge_if_all_discussions_are_resolved": true,"remove_source_branch_after_merge": true,"printing_merge_request_link_enabled": true,"merge_method": "merge","suggestion_commit_message": "","auto_devops_enabled": false,"auto_devops_deploy_strategy": "continuous","autoclose_referenced_issues": true,"repository_storage": "default","approvals_before_merge": 0,"mirror": false,"external_authorization_classification_label": null,"marked_for_deletion_at": null,"marked_for_deletion_on": null,"requirements_enabled": false,"compliance_frameworks": [],"permissions": {"project_access": null,"group_access": {"access_level": 50,"notification_level": 3}}}` var changesPending = `{"id":8312,"iid":102,"target_branch":"main","source_branch":"TestBranch","project_id":3771,"title":"Update somefile.yaml","state":"opened","created_at":"2023-03-14T13:43:17.895Z","updated_at":"2023-03-14T13:43:17.895Z","upvotes":0,"downvotes":0,"author":{"id":1755902,"name":"Luke Kysow","username":"lkysow","state":"active","avatar_url":"https://secure.gravatar.com/avatar/25fd57e71590fe28736624ff24d41c5f?s=80\\u0026d=identicon","web_url":"https://gitlab.com/lkysow"},"assignee":null,"assignees":[],"reviewers":[],"source_project_id":3771,"target_project_id":3771,"labels":"","description":"","draft":false,"work_in_progress":false,"milestone":null,"merge_when_pipeline_succeeds":false,"detailed_merge_status":"checking","merge_error":"","merged_by":null,"merged_at":null,"closed_by":null,"closed_at":null,"subscribed":false,"sha":"cb86d70f464632bdfbe1bb9bc0f2f9d847a774a0","merge_commit_sha":"","squash_commit_sha":"","user_notes_count":0,"changes_count":"","should_remove_source_branch":false,"force_remove_source_branch":true,"allow_collaboration":false,"web_url":"https://gitlab.com/lkysow/atlantis-example/merge_requests/13","references":{"short":"!13","relative":"!13","full":"lkysow/atlantis-example!13"},"discussion_locked":false,"changes":[],"user":{"can_merge":true},"time_stats":{"human_time_estimate":"","human_total_time_spent":"","time_estimate":0,"total_time_spent":0},"squash":false,"pipeline":null,"head_pipeline":null,"diff_refs":{"base_sha":"","head_sha":"","start_sha":""},"diverged_commits_count":0,"rebase_in_progress":false,"approvals_before_merge":0,"reference":"!13","first_contribution":false,"task_completion_status":{"count":0,"completed_count":0},"has_conflicts":false,"blocking_discussions_resolved":true,"overflow":false,"merge_status":"checking"}` var changesAvailable = `{"id":8312,"iid":102,"target_branch":"main","source_branch":"TestBranch","project_id":3771,"title":"Update somefile.yaml","state":"opened","created_at":"2023-03-14T13:43:17.895Z","updated_at":"2023-03-14T13:43:59.978Z","upvotes":0,"downvotes":0,"author":{"id":1755902,"name":"Luke Kysow","username":"lkysow","state":"active","avatar_url":"https://secure.gravatar.com/avatar/25fd57e71590fe28736624ff24d41c5f?s=80\\u0026d=identicon","web_url":"https://gitlab.com/lkysow"},"assignee":null,"assignees":[],"reviewers":[],"source_project_id":3771,"target_project_id":3771,"labels":[],"description":"","draft":false,"work_in_progress":false,"milestone":null,"merge_when_pipeline_succeeds":false,"detailed_merge_status":"not_approved","merge_error":"","merged_by":null,"merged_at":null,"closed_by":null,"closed_at":null,"subscribed":false,"sha":"cb86d70f464632bdfbe1bb9bc0f2f9d847a774a0","merge_commit_sha":null,"squash_commit_sha":null,"user_notes_count":0,"changes_count":"1","should_remove_source_branch":null,"force_remove_source_branch":true,"allow_collaboration":false,"web_url":"https://gitlab.com/lkysow/atlantis-example/merge_requests/13","references":{"short":"!13","relative":"!13","full":"lkysow/atlantis-example!13"},"discussion_locked":null,"changes":[{"old_path":"somefile.yaml","new_path":"somefile.yaml","a_mode":"100644","b_mode":"100644","diff":"--- a/somefile.yaml\\ +++ b/somefile.yaml\\ @@ -1 +1 @@\\ -gud\\ +good","new_file":false,"renamed_file":false,"deleted_file":false}],"user":{"can_merge":true},"time_stats":{"human_time_estimate":null,"human_total_time_spent":null,"time_estimate":0,"total_time_spent":0},"squash":false,"pipeline":null,"head_pipeline":null,"diff_refs":{"base_sha":"67cb91d3f6198189f433c045154a885784ba6977","head_sha":"cb86d70f464632bdfbe1bb9bc0f2f9d847a774a0","start_sha":"67cb91d3f6198189f433c045154a885784ba6977"},"approvals_before_merge":null,"reference":"!13","task_completion_status":{"count":0,"completed_count":0},"has_conflicts":false,"blocking_discussions_resolved":true,"overflow":false,"merge_status":"can_be_merged"}` +var headPipelineNotAvailable = `{"id": 22461274,"iid": 13,"project_id": 4580910,"title": "Update main.tf","description": "","state": "opened","created_at": "2019-01-15T18:27:29.375Z","updated_at": "2019-01-25T17:28:01.437Z","merged_by": null,"merged_at": null,"closed_by": null,"closed_at": null,"target_branch": "patch-1","source_branch": "patch-1-merger","user_notes_count": 0,"upvotes": 0,"downvotes": 0,"author": {"id": 1755902,"name": "Luke Kysow","username": "lkysow","state": "active","avatar_url": "https://secure.gravatar.com/avatar/25fd57e71590fe28736624ff24d41c5f?s=80\u0026d=identicon","web_url": "https://gitlab.com/lkysow"},"assignee": null,"reviewers": [],"source_project_id": 4580910,"target_project_id": 4580910,"labels": [],"work_in_progress": false,"milestone": null,"merge_when_pipeline_succeeds": false,"merge_status": "can_be_merged","detailed_merge_status": "mergeable","sha": "cb86d70f464632bdfbe1bb9bc0f2f9d847a774a0","merge_commit_sha": null,"squash_commit_sha": null,"discussion_locked": null,"should_remove_source_branch": null,"force_remove_source_branch": true,"reference": "!13","references": {"short": "!13","relative": "!13","full": "lkysow/atlantis-example!13"},"web_url": "https://gitlab.com/lkysow/atlantis-example/merge_requests/13","time_stats": {"time_estimate": 0,"total_time_spent": 0,"human_time_estimate": null,"human_total_time_spent": null},"squash": true,"task_completion_status": {"count": 0,"completed_count": 0},"has_conflicts": false,"blocking_discussions_resolved": true,"approvals_before_merge": null,"subscribed": false,"changes_count": "1","latest_build_started_at": "2019-01-15T18:27:29.375Z","latest_build_finished_at": "2019-01-25T17:28:01.437Z","first_deployed_to_production_at": null,"pipeline": {"id": 488598,"sha": "67cb91d3f6198189f433c045154a885784ba6977","ref": "patch-1-merger","status": "success","created_at": "2019-01-15T18:27:29.375Z","updated_at": "2019-01-25T17:28:01.437Z","web_url": "https://gitlab.com/lkysow/atlantis-example/-/pipelines/488598"},"head_pipeline": null,"diff_refs": {"base_sha": "67cb91d3f6198189f433c045154a885784ba6977","head_sha": "cb86d70f464632bdfbe1bb9bc0f2f9d847a774a0","start_sha": "67cb91d3f6198189f433c045154a885784ba6977"},"merge_error": null,"first_contribution": false,"user": {"can_merge": true}}`