Enhancements Gitlab Merge Request approvers

[gregory-v]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
• If you are interested in working on this issue or have submitted a pull request, please leave a comment.

---

User story

As a specific user for gitlab, I want approve a merge request like a validation before the requester apply the merge request via "atlantis apply" in comment.

Behaviour

At the moment the code used for the approver validation use the field ApprovalsLeft with the condition approvals.ApprovalsLeft > 0
https://github.com/runatlantis/atlantis/blob/main/server/events/vcs/gitlab_client.go#L197
Maybe it's depending on edition but ApprovalsLeft value is always 0.

• Gitlab version

https://git.organization.com/api/v4/version :
{
  "version": "15.11.2-ee",
  "revision": "916d24d1e48",
  "kas": {
    "enabled": true,
    "externalUrl": "",
    "version": "v15.11.0"
  },
  "enterprise": true
}

• MR not approved yet

https://git.organization.com/api/v4/projects/965/merge_requests/28/approvals : 
{
  "id": 1,
  "iid": 28,
  "project_id": 965,
  "title": "Atlantis: test",
  "description": "Atlantis: test",
  "state": "opened",
  "created_at": "2023-06-08T15:09:54.902Z",
  "updated_at": "2023-06-08T15:09:56.513Z",
  "merge_status": "can_be_merged",
  "approved": true,
  "approvals_required": 0,
  "approvals_left": 0,
  "require_password_to_approve": false,
  "approved_by": [],
  "suggested_approvers": [],
  "approvers": [],
  "approver_groups": [],
  "user_has_approved": false,
  "user_can_approve": false,
  "approval_rules_left": [],
  "has_approval_rules": false,
  "merge_request_approvers_available": false,
  "multiple_approval_rules_available": false,
  "invalid_approvers_rules": []
}

• gregory-v approved this merge request just now

{
  "id": 1,
  "iid": 28,
  "project_id": 965,
  "title": "Atlantis: test",
  "description": "Atlantis: test",
  "state": "opened",
  "created_at": "2023-06-08T15:09:54.902Z",
  "updated_at": "2023-06-08T15:13:19.788Z",
  "merge_status": "can_be_merged",
  "approved": true,
  "approvals_required": 0,
  "approvals_left": 0,
  "require_password_to_approve": false,
  "approved_by": [
    {
      "user": {
        "id": 311,
        "username": "gregory-v",
        "name": "Gregory V",
        "state": "active",
        "avatar_url": "",
        "web_url": "https://git.organization.com/gregory-v"
      }
    }
  ],
  "suggested_approvers": [],
  "approvers": [],
  "approver_groups": [],
  "user_has_approved": true,
  "user_can_approve": false,
  "approval_rules_left": [],
  "has_approval_rules": false,
  "merge_request_approvers_available": false,
  "multiple_approval_rules_available": false,
  "invalid_approvers_rules": []
}

• Approve additionally by s-atlantis

{
  "id": 1,
  "iid": 28,
  "project_id": 965,
  "title": "Atlantis: test",
  "description": "Atlantis: test",
  "state": "opened",
  "created_at": "2023-06-08T15:09:54.902Z",
  "updated_at": "2023-06-08T15:26:23.225Z",
  "merge_status": "can_be_merged",
  "approved": true,
  "approvals_required": 0,
  "approvals_left": 0,
  "require_password_to_approve": false,
  "approved_by": [
    {
      "user": {
        "id": 311,
        "username": "gregory-v",
        "name": "Gregory V",
        "state": "active",
        "avatar_url": "",
        "web_url": "https://git.organization.com/gregory-v"
      }
    },
    {
      "user": {
        "id": 383,
        "username": "s-atlantis",
        "name": "Atlantis",
        "state": "active",
        "avatar_url": "",
        "web_url": "https://git.organization.com/s-atlantis"
      }
    }
  ],
  "suggested_approvers": [],
  "approvers": [],
  "approver_groups": [],
  "user_has_approved": true,
  "user_can_approve": false,
  "approval_rules_left": [],
  "has_approval_rules": false,
  "merge_request_approvers_available": false,
  "multiple_approval_rules_available": false,
  "invalid_approvers_rules": []
}

Solution

Instead using the ApprovalsLeft field we could use the field ApprovedBy and looking for the username in an environnement variable in order to find one of approvers

ATLANTIS_GITLAB_APPROVERS_LIST="gregory-v,eric-b"

• Existing code
  https://github.com/runatlantis/atlantis/blob/main/server/events/vcs/gitlab_client.go#L192

• Code sample with environnement variable

// PullIsApproved returns true if the merge request was approved.
func (g *GitlabClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (approvalStatus models.ApprovalStatus, err error) {
	//FIXME use UserConfig variable instead os.Getenv...
        gitlabApproverslist := os.Getenv("ATLANTIS_GITLAB_APPROVERS_LIST")
	approvals, _, err := g.Client.MergeRequests.GetMergeRequestApprovals(repo.FullName, pull.Num)
	if err != nil {
		return approvalStatus, err
	}

	//No approver in gitlabApproverslist check ApprovalsLeft like previously
	if len(gitlabApproverslist) == 0 {
		if approvals.ApprovalsLeft > 0 {
			return approvalStatus, nil
		} else {
			return models.ApprovalStatus{
				IsApproved: true,
			}, nil
		}
	}
	//Gitlab api return list of approved_by
	if len(approvals.ApprovedBy) > 0 {
		for _, user := range approvals.ApprovedBy {
			if pull.Author != user.User.Username && utils.SlicesContains(strings.Split(gitlabApproverslist, ","), user.User.Username) {
				return models.ApprovalStatus{
					IsApproved: true,
					ApprovedBy: user.User.Name,
				}, nil
			}
		}
	}
	return approvalStatus, nil
}

Alternatives

Still with the ApprovedBy field we could looking for one of approver in atlantis.yaml file, we could set approvers list by directory ?