runatlantis / atlantis

Terraform Pull Request Automation
https://www.runatlantis.io
Other
7.68k stars 1.05k forks source link

--gh-allow-mergeable-bypass-apply for multiple servers #2848

Open Dawnflash opened 1 year ago

Dawnflash commented 1 year ago

Community Note


Describe the user story Our setup:

When testing for mergeability, the function here only checks its own apply status but ignores those of other servers, causing all of them to report mergeable as failed.

Desired behavior: the servers ignore the apply status checks of all the servers and pass the mergeable check.

Describe the solution you'd like You could allow deepening the hierarchy of the status check names (instead of <vcs_status_name/<cmd_name>: use <vcs_status_name>/<cmd_name> [<vcs_server_name>]:, eg. atlantis/apply [serverA]: and then you can keep using the prefix match that's present in the code right now.

This would involve a new configuration flag like vcs-server-name or similar that defaults to empty string and isn't used if not specified (the check name prefix would default to <vcs_status_name/<cmd_name>:).

Describe the drawbacks of your solution This collides somewhat with the original concept of vcs-status-name which was supposed to set instances apart and could be confusing going forwards. The solution assumes multiple servers sharing vcs-status-name and being set apart by a different flag.

Describe alternatives you've considered

Add a configuration flag specifying a list of vcs_status_names to ignore. Eg. a configuration flag --gh-mergeable-bypass-names that can be set to something like ["atlantis", "atlantis-A"] on all the servers. It's not particularly pretty or scalable but it would do the job.

Other ways of dealing with the flag are possible, eg. a flag like vcs_status_group_name which could be an optional flag that changes the status check prefix or adds a needle in it to look for. Eg. when set to mygroup it turns the status check names into mygroup/<vcs_status_name>/apply and the prefix check is altered to take this into consideration.

Maybe even a change in the status check naming, changing it to eg. atlantis/<CMD> [vcs-status-name] but that's backwards incompatible and could break people's branch protection settings.

nitrocode commented 1 year ago

this might be a tricky implementation.

Let me cc others for their thoughts

nitrocode commented 1 year ago

@Dawnflash does using --executable-name from 0.22.3 reduce or resolve this problem?

Dawnflash commented 1 year ago

It sidesteps the problem, trades it for having to execute actions twice in some cases. I don't mind but we're all about self-service and this would confuse the heck out of devs. I don't think separate executable names are an option for us. Yet, it's a great alternative for people who require mergeable. We just downgraded to approved for the time being.

bakayolo commented 3 weeks ago

+1 to this, any workaround that would allow us to use mergeable requirement ?