fix(gitlab): Add backwards compatible mergability check #3277

jukie · 2023-03-29T19:50:55Z

what

Adds a conditional for whether the gitlab client version supports detailed_merge_status (anything 15.6+) and will use the old merge_status if it doesn't.

why

In newer versions of Atlantis, apply requirements won't work if users are running older versions of gitlab.

tests

I don't have a gitlab server <15.6 to test with but I've reviewed the api response data and updated tests accordingly

references

Closes apply fails with apply_requirements mergeable on gitlab < 15.6 #3258
Relates to Gitlab, double planning message #3089

jukie · 2023-03-29T19:53:51Z

server/events/vcs/gitlab_client_test.go

@@ -408,8 +429,8 @@ func TestGitlabClient_MarkdownPullLink(t *testing.T) {
 	Equals(t, exp, s)
 }

-var mergeSuccess = `{"id":22461274,"iid":13,"project_id":4580910,"title":"Update main.tf","description":"","state":"merged","created_at":"2019-01-15T18:27:29.375Z","updated_at":"2019-01-25T17:28:01.437Z","merged_by":{"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"},"merged_at":"2019-01-25T17:28:01.459Z","closed_by":null,"closed_at":null,"target_branch":"patch-1","source_branch":"patch-1-merger","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,"source_project_id":4580910,"target_project_id":4580910,"labels":[],"work_in_progress":false,"milestone":null,"merge_when_pipeline_succeeds":false,"detailed_merge_status":"mergeable","sha":"cb86d70f464632bdfbe1bb9bc0f2f9d847a774a0","merge_commit_sha":"c9b336f1c71d3e64810b8cfa2abcfab232d6bff6","user_notes_count":0,"discussion_locked":null,"should_remove_source_branch":null,"force_remove_source_branch":false,"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":false,"subscribed":true,"changes_count":"1","latest_build_started_at":null,"latest_build_finished_at":null,"first_deployed_to_production_at":null,"pipeline":null,"diff_refs":{"base_sha":"67cb91d3f6198189f433c045154a885784ba6977","head_sha":"cb86d70f464632bdfbe1bb9bc0f2f9d847a774a0","start_sha":"67cb91d3f6198189f433c045154a885784ba6977"},"merge_error":null,"approvals_before_merge":null}`


What I've done here is add "merge_status":"can_be_merged" to the testdata response. If preferred I can add a fully separate dataset instead.

I will say maybe add a fully separated dataset instead

Oh missed this comment. Agreed. It would be better to just put it in a separate pretty formatted json file. We do separate supporting test files for terraform tests so json wouldn't be any different

server/events/vcs/gitlab_client.go

nitrocode · 2023-03-29T20:49:03Z

@jukie do you mind pairing with someone in issue #3258 like @zerog2k and having them verify that this change will work as expected for older versions since you do not have a version to test with?

If you take your feature branch and merge it with your HEAD branch of your fork, new atlantis container will be built and pushed to your fork. This new image can then be used by anyone to verify the change in this pr works as expected.

jukie · 2023-03-29T21:59:09Z

@zerog2k if you could check with either of the following images:

docker pull ghcr.io/jukie/atlantis:dev-alpine-bb0882f https://github.com/jukie/atlantis/pkgs/container/atlantis/81433423?tag=dev-alpine-bb0882f
docker pull ghcr.io/jukie/atlantis:dev-debian-bb0882f https://github.com/jukie/atlantis/pkgs/container/atlantis/81433423?tag=dev-debian-bb0882f

nitrocode · 2023-03-30T02:30:56Z

cc @skruger ^

nitrocode · 2023-03-30T02:35:47Z

Also from #3089 cc @rhysm @mustdiechik

zerog2k · 2023-03-30T19:51:54Z

docker pull ghcr.io/jukie/atlantis:dev-alpine-bb0882f

Thanks @jukie appears to be working for me on gitlab ee 14.10.5 and your build

jukie · 2023-03-30T23:50:52Z

@nitrocode looks like we're good here then? I'll get the test case updated though.

nitrocode · 2023-03-31T00:32:55Z

Thank you @jukie @zerog2k !

I'm 20 minutes this will spin up a new build and save the new image as dev. Feel free to use that image until v0.23.4 is released.

d-cryptic · 2023-03-31T07:51:37Z

I tried the dev image, I am still facing the same issue #3270
Will it only work for mergeable argument as of now? Will it work for approved too?

jukie · 2023-03-31T17:45:02Z

@d-cryptic the only API change between these versions was detailed merge status vs merge status so functionality for approvals should be the same.
Could you try adding --require-approval to your server startup flags?

d-cryptic · 2023-04-01T11:20:52Z

I did, but it's not working !!

nitrocode · 2023-04-01T14:43:22Z

@d-cryptic i know this is frustrating for you. Please remember that we're all doing this as volunteer work so please try to keep your messages as helpful as possible to reduce the back and forth.

For a developer to fix this, can you post error logs and anything that may be helpful in #3270 (the issue you first created)? I have reopened the ticket.

jukie · 2023-04-01T15:46:54Z

Check my note in #3270 (comment)

@d-cryptic is running gitlab community edition so requiring approvals before merge isn't an option.

jamengual · 2023-04-01T15:50:16Z

maybe we should add that to the docs

…

On Sat, Apr 1, 2023, 8:47 a.m. Isaac Wilson ***@***.***> wrote: Check my note in #3270 (comment) <#3270 (comment)> @d-cryptic <https://github.com/d-cryptic> is running gitlab community edition so requiring approvals before merge isn't an option. — Reply to this email directly, view it on GitHub <#3277 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQ3ERDBQBB2G43277QTL5DW7BEXVANCNFSM6AAAAAAWMKZ6VY> . You are receiving this because your review was requested.Message ID: ***@***.***>

jukie · 2023-04-01T19:02:03Z

I can add a note within the Gitlab section of https://www.runatlantis.io/docs/command-requirements.html#approved

Also wondering the best place in code is to add a check via https://docs.gitlab.com/ee/api/metadata.html for the value of enterprise and log whether approve requirements are supported.

jukie · 2023-04-01T19:02:41Z

I'll open another WIP PR and we'll discuss there

jamengual · 2023-04-01T19:16:54Z

sounds good

…

On Sat, Apr 1, 2023, 12:02 p.m. Isaac Wilson ***@***.***> wrote: I'll open another WIP PR and we'll discuss there — Reply to this email directly, view it on GitHub <#3277 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQ3ERCWXC2V2ZGY5O3C5ITW7B3VXANCNFSM6AAAAAAWMKZ6VY> . You are receiving this because your review was requested.Message ID: ***@***.***>

d-cryptic · 2023-04-03T11:05:52Z

Check my note in #3270 (comment)

@d-cryptic is running gitlab community edition so requiring approvals before merge isn't an option.

So in community edition, approved requirement won't work right?

jukie · 2023-04-04T05:28:44Z

Correct @d-cryptic.

Looks like there was already a notice in the docs as well so I'm not sure we need any updates there -

atlantis/runatlantis.io/docs/command-requirements.md

Line 263 in 089dc1a

    
           * For more information on GitLab merge request reviews and approvals (only supported on GitLab Enterprise) see: [https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html).

* Add backwards compatibility for merability check in Gitlab client * Cleanup

…3277) * Add backwards compatibility for merability check in Gitlab client * Cleanup

Add backwards compatibility for merability check in Gitlab client

67ba874

jukie requested a review from a team as a code owner March 29, 2023 19:50

github-actions bot added go Pull requests that update Go code provider/gitlab labels Mar 29, 2023

Cleanup

4cf1435

jukie commented Mar 29, 2023

View reviewed changes

server/events/vcs/gitlab_client.go Show resolved Hide resolved

nitrocode approved these changes Mar 29, 2023

View reviewed changes

nitrocode added this to the v0.23.4 milestone Mar 29, 2023

nitrocode changed the title ~~Add backwards compatibility for merability check in Gitlab client~~ fix(gitlab): Add backwards compatible merability check Mar 30, 2023

nitrocode changed the title ~~fix(gitlab): Add backwards compatible merability check~~ fix(gitlab): Add backwards compatible mergability check Mar 30, 2023

nitrocode merged commit 8adb001 into runatlantis:main Mar 31, 2023

nitrocode mentioned this pull request Apr 1, 2023

Gitlab: apply_requirements not working < 15.6 #3270

Closed

jukie deleted the jukie/fix-apply-requirements-gitlab branch April 1, 2023 15:47

jukie mentioned this pull request Apr 4, 2023

Add note in docs about needing gitlab ee for approval requirements #3290

Closed

bakeemawaytoys mentioned this pull request Apr 14, 2023

Nil pointer error when manually running "atlantis plan" on Gitlab after upgrading Atlantis to 0.23.2 #3196

Closed

nitrocode pushed a commit that referenced this pull request May 5, 2023

fix(gitlab): Add backwards compatible mergability check (#3277)

ae85080

* Add backwards compatibility for merability check in Gitlab client * Cleanup

albertorm95 mentioned this pull request May 19, 2023

fix(metrics): add missing stat metric scopes #3416

Merged

syst0m mentioned this pull request Aug 31, 2023

Apply fails on Gitlab 16.4.0 with "Pull request must be mergeable before running apply." #3722

Open

ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024

fix(gitlab): Add backwards compatible mergability check (runatlantis#…

eadda68

…3277) * Add backwards compatibility for merability check in Gitlab client * Cleanup

ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024

fix(gitlab): Add backwards compatible mergability check (runatlantis#…

4bf2c9f

…3277) * Add backwards compatibility for merability check in Gitlab client * Cleanup

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(gitlab): Add backwards compatible mergability check #3277

fix(gitlab): Add backwards compatible mergability check #3277

jukie commented Mar 29, 2023 •

edited by nitrocode

Loading

jukie Mar 29, 2023

jamengual Mar 29, 2023

nitrocode Apr 1, 2023

nitrocode commented Mar 29, 2023

jukie commented Mar 29, 2023 •

edited

Loading

nitrocode commented Mar 30, 2023

nitrocode commented Mar 30, 2023

zerog2k commented Mar 30, 2023

jukie commented Mar 30, 2023

nitrocode commented Mar 31, 2023

d-cryptic commented Mar 31, 2023 •

edited

Loading

jukie commented Mar 31, 2023

d-cryptic commented Apr 1, 2023

nitrocode commented Apr 1, 2023

jukie commented Apr 1, 2023

jamengual commented Apr 1, 2023 via email

jukie commented Apr 1, 2023

jukie commented Apr 1, 2023

jamengual commented Apr 1, 2023 via email

d-cryptic commented Apr 3, 2023

jukie commented Apr 4, 2023

fix(gitlab): Add backwards compatible mergability check #3277

fix(gitlab): Add backwards compatible mergability check #3277

Conversation

jukie commented Mar 29, 2023 • edited by nitrocode Loading

what

why

tests

references

jukie Mar 29, 2023

Choose a reason for hiding this comment

jamengual Mar 29, 2023

Choose a reason for hiding this comment

nitrocode Apr 1, 2023

Choose a reason for hiding this comment

nitrocode commented Mar 29, 2023

jukie commented Mar 29, 2023 • edited Loading

nitrocode commented Mar 30, 2023

nitrocode commented Mar 30, 2023

zerog2k commented Mar 30, 2023

jukie commented Mar 30, 2023

nitrocode commented Mar 31, 2023

d-cryptic commented Mar 31, 2023 • edited Loading

jukie commented Mar 31, 2023

d-cryptic commented Apr 1, 2023

nitrocode commented Apr 1, 2023

jukie commented Apr 1, 2023

jamengual commented Apr 1, 2023 via email

jukie commented Apr 1, 2023

jukie commented Apr 1, 2023

jamengual commented Apr 1, 2023 via email

d-cryptic commented Apr 3, 2023

jukie commented Apr 4, 2023

jukie commented Mar 29, 2023 •

edited by nitrocode

Loading

jukie commented Mar 29, 2023 •

edited

Loading

d-cryptic commented Mar 31, 2023 •

edited

Loading