feat: add ignore-vcs-status-names (github only)

[bakayolo]

what

Comma separated list of VCS status names from other atlantis services.

When gh-allow-mergeable-bypass-apply is true, will ignore status checks (e.g. status1/plan, status1/apply, status2/plan, status2/apply) from other atlantis services when checking if the PR is mergeable.

Only for Github but can be extended to other VCS in the future.

why

This PR is a proposal to close #2848

tests

I have only run make test but planning on testing in my own environment if the proposal is accepted.
Tested with make test and on my environment with bappr/atlantis:cud-ignore-vcs-status-names-ca1171e0dd174edbbd63f8f6ae560caa6af4c0c7-0.

references

closes #2848

[jamengual]

interesting proposition.
one quick thing I will say that instead of adding a flag for this it will be much better to add it to the server side repo config with something like :

vcs-status-to-ignore:
  - name1
  - name2
  - etc

[bakayolo]

@jamengual thanks for the feedback, why do you think it is much better? I would actually imagine that having other-vcs-status-names-to-ignore close to vcs-status-name would be less confusing no?

[jamengual]

Because we have a trillion flags, and we want to avoid having more that is the main reason, plus if is moved there then you can have different repos regexes with different statuses.

[bakayolo]

@jamengual I've been looking at the code and I believe it's gonna be pretty complex to do

plus if is moved there then you can have different repos regexes with different statuses.

because the FetchPullStatus is not called at the repo config level. It's basically a sort of boolean ("has been approved" and "is mergeable", no conditions)

Also, I am struggling to understand the benefit of having different repos regexes with different statuses. Simply put, #2848 is basically a bug where multiple atlantis instances with different vcs status name conflict with each other and forbid the apply when mergeable is set. Which means that if we do not make atlantis aware of ALL the others atlantis running for the repo, they will continue conflicting with each others. So I think that config should be global.

Wdyt?

[jamengual]

Sorry, this is a standard ask we ask everyone.
I did not look at the code to see if the context will be present at that point in the execution for you to do it.
As you mentioned, it could get pretty complex, so keeping the flag then is better.

[anryko]

Small nitpick, other in the name seems redundant. The vcs-status-names-to-ignore would be sufficient, but considering the pattern in other arguments naming (hide-, silence-, include-, allow-), the name should resemble ingnore-vsc-status-names.

[bakayolo]

FYI, I've tested this change on my environment (Dev, Staging, prod) with bappr/atlantis:cud-ignore-vcs-status-names-ca1171e0dd174edbbd63f8f6ae560caa6af4c0c7-0.

[bakayolo]

Hey @jamengual, do you know when we can get this PR reviewed? It’s been running fine on 20 of my atlantis servers for the past 2 weeks and working as expected. I’d like to deploy it to my other 50 ones. But I would prefer to use your upstream version instead, versus my own docker image. Thanks.

[bakayolo]

@X-Guardian thx, this is done.

[X-Guardian]

The documentation needs updating to add this new flag here: https://github.com/runatlantis/atlantis/blob/main/runatlantis.io/docs/server-configuration.md

[bakayolo]

@X-Guardian done as well. Thanks.

[X-Guardian]

Nearly there @bakayolo. Just need to get the docs refined.

[bakayolo]

@X-Guardian thanks. Done.

Not sure this description is quite right. they might not be other Atlantis servers.

The description is actually right. This PR only ignore vcs status names such as something/plan and something/apply.

[X-Guardian]

Thanks @bakayolo. Just a couple of small refinements and we are good to merge.

[bakayolo]

Thanks @X-Guardian ! Should be good! 👍

[X-Guardian]

@bakayolo, can you resolve the merge conflicts?

[bakayolo]

@X-Guardian all good, thanks!

[X-Guardian]

Thanks for this @bakayolo. You can test it using one of these container images: dev-alpine-841f7b4 or dev-debian-841f7b4

[bakayolo]

Nice! Thanks!