Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gitlab): Prevent nil pointer dereference when HeadPipeline is empty #3428

Merged
merged 1 commit into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/e2e-deps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions server/events/vcs/gitlab_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,14 +280,23 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
return false, err
}

// Prevent nil pointer error when mr.HeadPipeline is empty
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test for this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is at

func TestGitlabClient_PullIsMergeable(t *testing.T) {
. Feel free to ping me directly if you need assistance building the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just haven't got the time yet. I'm gonna try doing this over the long weekend here and get back to you

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GenPage finally got the time to work on this, the tests are here, could you review them?

// 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 {
return false, err
}

// 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
}
Expand All @@ -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()
Expand Down
37 changes: 36 additions & 1 deletion server/events/vcs/gitlab_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}
Expand All @@ -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
Expand Down Expand Up @@ -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)
})
Expand Down Expand Up @@ -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}}`