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

Incorrect GitLab mergeable status determination #1174

Closed
gmaghera opened this issue Sep 1, 2020 · 13 comments
Closed

Incorrect GitLab mergeable status determination #1174

gmaghera opened this issue Sep 1, 2020 · 13 comments
Labels
bug Something isn't working provider/gitlab waiting-on-response Waiting for a response from the user

Comments

@gmaghera
Copy link
Contributor

gmaghera commented Sep 1, 2020

Hello,

We have Atlantis set up and configured with gitlab.com. On the repo side I have the apply requirement set to mergeable.

The docs state:
"GitLab
For GitLab, a merge request will be mergeable if it has no conflicts and if all required approvers have approved the pull request."

That does not seem to be the case however. I can atlantis apply a non-mergeable MR, and I confirmed that the atlantis pod is checking mergeable status:

image

atlantis-0 atlantis 2020/09/01 16:31:46+0000 [INFO] rivian/dc/dc-terraform#137: Pull request mergeable status: true
atlantis-0 atlantis 2020/09/01 16:31:46+0000 [INFO] rivian/dc/dc-terraform#137: Cannot determine which version to use from terraform configuration, detected 0 possibilities.
atlantis-0 atlantis 2020/09/01 16:31:46+0000 [INFO] rivian/dc/dc-terraform#137: Successfully ran "echo \"terraform0.12.29\"" in "/atlantis-data/repos/rivian/dc/dc-terraform/137/default/tools/us-east-1/debug"
atlantis-0 atlantis 2020/09/01 16:32:08+0000 [INFO] rivian/dc/dc-terraform#137: Successfully ran "terragrunt apply -no-color $PLANFILE" in "/atlantis-data/repos/rivian/dc/dc-terraform/137/default/tools/us-east-1/debug"

I can try setting it to approved, but I would prefer to use mergeable, as our process requires two approval for a MR to be considered mergeable.

Thanks for looking into this!

Cheers,
Gabor

@gmaghera
Copy link
Contributor Author

gmaghera commented Sep 1, 2020

I've done some digging and this appears to be an issue with the GitLab API, or perhaps Atlantis should be checking additional fields in the response JSON.

if mr.MergeStatus == "can_be_merged" && mr.ApprovalsBeforeMerge <= 0 {
return true, nil
}

if mr.MergeStatus == "can_be_merged" && mr.ApprovalsBeforeMerge <= 0 {
		return true, nil
}

GitLab API response for a non-mergeable MR, https://gitlab.com/rivian/dc/dc-terraform/-/merge_requests/142.

❯ curl --silent --location --request GET "https://gitlab.com/api/v4/projects/19594670/merge_requests/142" \
--header "Private-Token: ${PRIVATE_TOKEN}" | jq '"merge_status =" + .merge_status + "; approvals_before_merge = " + .approvals_before_merge'
"merge_status =can_be_merged; approvals_before_merge = "

Here's a screenshot of the MR, which shows that it requires two approvals before merging:
image

@lkysow
Copy link
Member

lkysow commented Sep 1, 2020

What's the output of your

❯ curl --silent --location --request GET "https://gitlab.com/api/v4/projects/19594670/merge_requests/142" \
--header "Private-Token: ${PRIVATE_TOKEN}" | jq '"merge_status =" + .merge_status + "; approvals_before_merge = " + .approvals_before_merge'
"merge_status =can_be_merged; approvals_before_merge = "

?

@lkysow lkysow added the bug Something isn't working label Sep 1, 2020
@gmaghera
Copy link
Contributor Author

gmaghera commented Sep 2, 2020

Hi @lkysow. Thanks for looking into this.

The output is part that snippet I shared earlier, it's just difficult to discern.

This is the command:

❯ curl --silent --location --request GET "https://gitlab.com/api/v4/projects/19594670/merge_requests/142" \
--header "Private-Token: ${PRIVATE_TOKEN}" | jq '"merge_status =" + .merge_status + "; approvals_before_merge = " + .approvals_before_merge'

And this is the resulting output:

"merge_status =can_be_merged; approvals_before_merge = "

Looking at the Atlantis code this appears to be sufficient to deem the MR mergeable. That API response is also conflicting with what I see when I look at the same MR via the GitLab UI, which shows it as unmergeable with 2 required approvals before the merge. I reached out to GitLab for clarification, and for the meantime I'm using "approved" status as the apply requirement.

@lkysow
Copy link
Member

lkysow commented Sep 2, 2020

So approvals before merge is null? Maybe you can just paste the whole api response output?

@gmaghera
Copy link
Contributor Author

gmaghera commented Sep 2, 2020

Yes, it is null.

{
  "id": 69441072,
  "iid": 142,
  "project_id": 19594670,
  "title": "Feature/atlantis and tg dependencies",
  "description": "",
  "state": "closed",
  "created_at": "2020-09-01T18:38:33.264Z",
  "updated_at": "2020-09-01T21:57:06.913Z",
  "merged_by": null,
  "merged_at": null,
  "closed_by": {
    "id": 447938,
    "name": "Gabor Maghera",
    "username": "gmaghera",
    "state": "active",
    "avatar_url": "https://secure.gravatar.com/avatar/03d14e0c54a725785a3344aa07d1673f?s=80&d=identicon",
    "web_url": "https://gitlab.com/gmaghera"
  },
  "closed_at": "2020-09-01T21:57:06.929Z",
  "target_branch": "master",
  "source_branch": "feature/atlantis-and-tg-dependencies",
  "user_notes_count": 7,
  "upvotes": 0,
  "downvotes": 0,
  "author": {
    "id": 447938,
    "name": "Gabor Maghera",
    "username": "gmaghera",
    "state": "active",
    "avatar_url": "https://secure.gravatar.com/avatar/03d14e0c54a725785a3344aa07d1673f?s=80&d=identicon",
    "web_url": "https://gitlab.com/gmaghera"
  },
  "assignees": [],
  "assignee": null,
  "source_project_id": 19594670,
  "target_project_id": 19594670,
  "labels": [],
  "work_in_progress": false,
  "milestone": null,
  "merge_when_pipeline_succeeds": false,
  "merge_status": "can_be_merged",
  "sha": "2c5a3d1c254eb3351f1c35f76a7bd046e73540cd",
  "merge_commit_sha": null,
  "squash_commit_sha": null,
  "discussion_locked": null,
  "should_remove_source_branch": null,
  "force_remove_source_branch": true,
  "reference": "!142",
  "references": {
    "short": "!142",
    "relative": "!142",
    "full": "rivian/dc/dc-terraform!142"
  },
  "web_url": "https://gitlab.com/rivian/dc/dc-terraform/-/merge_requests/142",
  "time_stats": {
    "time_estimate": 0,
    "total_time_spent": 0,
    "human_time_estimate": null,
    "human_total_time_spent": null
  },
  "squash": false,
  "task_completion_status": {
    "count": 0,
    "completed_count": 0
  },
  "has_conflicts": false,
  "blocking_discussions_resolved": true,
  "approvals_before_merge": null,
  "subscribed": true,
  "changes_count": "2",
  "latest_build_started_at": null,
  "latest_build_finished_at": "2020-09-01T19:41:56.333Z",
  "first_deployed_to_production_at": null,
  "pipeline": {
    "id": 184503216,
    "sha": "2c5a3d1c254eb3351f1c35f76a7bd046e73540cd",
    "ref": "feature/atlantis-and-tg-dependencies",
    "status": "success",
    "created_at": "2020-09-01T19:40:44.013Z",
    "updated_at": "2020-09-01T19:41:56.334Z",
    "web_url": "https://gitlab.com/rivian/dc/dc-terraform/-/pipelines/184503216"
  },
  "head_pipeline": {
    "id": 184546055,
    "sha": "b57b0be0fa84771a470286e0ae854f24de0bb291",
    "ref": "feature/atlantis-and-tg-dependencies",
    "status": "failed",
    "created_at": "2020-09-01T22:02:16.190Z",
    "updated_at": "2020-09-01T22:19:28.782Z",
    "web_url": "https://gitlab.com/rivian/dc/dc-terraform/-/pipelines/184546055",
    "before_sha": "0000000000000000000000000000000000000000",
    "tag": false,
    "yaml_errors": null,
    "user": {
      "id": 447938,
      "name": "Gabor Maghera",
      "username": "gmaghera",
      "state": "active",
      "avatar_url": "https://secure.gravatar.com/avatar/03d14e0c54a725785a3344aa07d1673f?s=80&d=identicon",
      "web_url": "https://gitlab.com/gmaghera"
    },
    "started_at": null,
    "finished_at": "2020-09-01T22:19:28.781Z",
    "committed_at": null,
    "duration": null,
    "coverage": null,
    "detailed_status": {
      "icon": "status_failed",
      "text": "failed",
      "label": "failed",
      "group": "failed",
      "tooltip": "failed",
      "has_details": true,
      "details_path": "/rivian/dc/dc-terraform/-/pipelines/184546055",
      "illustration": null,
      "favicon": "https://gitlab.com/assets/ci_favicons/favicon_status_failed-41304d7f7e3828808b0c26771f0309e55296819a9beea3ea9fbf6689d9857c12.png"
    }
  },
  "diff_refs": {
    "base_sha": "4afe1596c04a7c79486135b012c72ad7cadb578b",
    "head_sha": "2c5a3d1c254eb3351f1c35f76a7bd046e73540cd",
    "start_sha": "05d6f7475ec11c3b333a46be31ea729b55f261ac"
  },
  "merge_error": null,
  "first_contribution": false,
  "user": {
    "can_merge": true
  }
}

@lkysow
Copy link
Member

lkysow commented Sep 2, 2020

Is it null for all your pull requests? Maybe gitlab's api has changed? Is it null for PRs that do have enough approvals?

@gmaghera
Copy link
Contributor Author

gmaghera commented Sep 3, 2020

Yes it's null consistently, even where the two necessary approvals have been met.

@gmaghera
Copy link
Contributor Author

gmaghera commented Sep 3, 2020

Luke, just so you have a full picture, here's how we have MR approvals set for our terraform/terragrunt mono-repo:

image

I've reached out to our company's GitLab support contact and they think it's a bug, and have opened an issue internally.

@gmaghera
Copy link
Contributor Author

Here is the issue GitLab opened with regard to the API response: https://gitlab.com/gitlab-org/gitlab/-/issues/246991

@jghal
Copy link

jghal commented Apr 26, 2022

The GitLab issue doesn't look particularly lively. I'm trying to get our account rep to add a "paid customer wants this" status to that issue. In the meantime does Atlantis support a mode where it will apply after merge so the GitLab configuration can be enforced, or perhaps a GitLab version of the Owners policy check?

@jghal
Copy link

jghal commented May 5, 2022

So there's a separate API call to check an MR's approval state which looks like it could be used here.

https://docs.gitlab.com/ee/api/merge_request_approvals.html#get-the-approval-state-of-merge-requests

Response for an an MR requiring 1 approval and is not approved

{
  "approval_rules_overwritten": false,
  "rules": [
    {
      "id": 12345,
      "name": "All Members",
      "rule_type": "any_approver",
      "eligible_approvers": [],
      "approvals_required": 1,
      "users": [],
      "groups": [],
      "contains_hidden_groups": false,
      "section": null,
      "source_rule": null,
      "overridden": false,
      "code_owner": false,
      "approved_by": [],
      "approved": false
    }
  ]
}

Response for an MR that is approved, and it looks like it includes the information to support the Owners policy check.

{
  "approval_rules_overwritten": false,
  "rules": [
    {
      "id": 234,
      "name": "All Members",
      "rule_type": "any_approver",
      "eligible_approvers": [],
      "approvals_required": 1,
      "users": [],
      "groups": [],
      "contains_hidden_groups": false,
      "section": null,
      "source_rule": null,
      "overridden": false,
      "code_owner": false,
      "approved_by": [
        {
          "id": 1234,
          "username": "...",
          "name": "Justin Georgeson",
          "state": "active",
          "avatar_url": "",
          "web_url": "https://gitlab.com/..."
        }
      ],
      "approved": true
    }
  ]
}

@jamengual
Copy link
Contributor

is this still an issue with v0.19.8?

@jamengual jamengual added the waiting-on-response Waiting for a response from the user label Aug 26, 2022
@jghal
Copy link

jghal commented Aug 26, 2022

I think between GitLab releasing 15.x and updates to xanzy-gitlab library this has been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working provider/gitlab waiting-on-response Waiting for a response from the user
Projects
None yet
Development

No branches or pull requests

5 participants