fix: Preventing merging without running atlantis apply on Gitlab

[shurkus]

what

Preventing merging without running atlantis apply on Gitlab

why

Preventing merging without running atlantis apply on Gitlab

tests

• I have tested my changes

references

#4372

[chenrui333]

I have fixed the tests per #4653 refreshed the pr and rerun

[chenrui333]

@shurkus please rebase your PR to the latest main branch. (I actually cannot refresh your branch due to permission denied issue)

For future pull requests, please allow maintainers to edit your PR to simplify the merge process.

[shurkus]

@chenrui333 rebased also allow edit

[chenrui333]

Thanks @shurkus!

[Unibozu]

Hi @shurkus @chenrui333

I wanted to highlight this feels like a breaking change in the way atlantis works with Gitlab Merge Requests, not a fix. This should be behind a feature flag, documented and fully tested (not just a "it works on my machine").

Especially as it's a weird workaround (the apply steps will always look like it's "running") with Gitlab lacking a way to enforce passing a certain CI job to merge an MR. The other difference is that on Github I can control which repositories require this enforcement, while this change will make all MRs on Gitlab requiring an apply before becoming mergeable.

Can we consider to revert the changes until this is better implemented?

[lukemassa]

I agree that this should be reverted as we come up with a more holistic solution here, for a few reasons:

1. This is a breaking change that changes behavior, not a fix
2. I'm not sure why this is implemented only for auto plans and not for comment plans
3. This makes it look like atlantis apply is "stuck", which is best case confusing, and worst case might have some other effects (e.g. interact poorly with mergeable)

I think one issue is the semantics of "pending" are different in gitlab and github, and I think we should either commit to unifying the two notions, or acknowledging they are different and document them differently.

I definitely want to address these issues, and maybe pre-emptively making "apply" be running is part of that solution, but I think it requires more discussion