apply_requirements not being evaluated #2605

Open theautoroboto opened 2 years ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Overview of the Issue

Using GitLab Community Edition [15.2.3] with Atlantis deployed on kubernetes using a statefulset, the apply_requirements settings defined in the environment variable ATLANTIS_REPO_CONFIG_JSON are not being evaluated.

Reproduction Steps

Deploy to kubernetes as a statefulset and define the environment variable. PRs are never checked for approval before allowing an apply comment to be posted and the atlantis apply process to be completed

- name: ATLANTIS_REPO_CONFIG_JSON
  value: '{"repos":[{"id":"/.*/", "apply_requirements":["approved"]}]}'

I have verified that the environment variable is present on the running pod

ATLANTIS_REPO_CONFIG_JSON={"repos":[{"id":"/.*/", "apply_requirements":["approved"]}]}

Logs

"level":"info","ts":"2022-10-13T13:51:14.477Z","caller":"events/instrumented_project_command_runner.go:53","msg":"plan success. output available at: https://xxxx/bsmith/runatlantis-terraform/-/merge_requests/12","json":{"repo":"bsmith/runatlantis-terraform","pull":"12"}}
{"level":"info","ts":"2022-10-13T13:55:08.158Z","caller":"events/events_controller.go:533","msg":"parsed comment as command=\"apply\" verbose=false dir=\"\" workspace=\"\" project=\"\" flags=\"\"","json":{}}
{"level":"info","ts":"2022-10-13T13:55:08.614Z","caller":"events/project_command_context_builder.go:313","msg":"detected module requires version: \"0.14.7\"","json":{"repo":"bsmith/runatlantis-terraform","pull":"12"}}
{"level":"info","ts":"2022-10-13T13:55:08.694Z","caller":"runtime/apply_step_runner.go:39","msg":"starting apply","json":{"repo":"bsmith/runatlantis-terraform","pull":"12"}}
{"level":"info","ts":"2022-10-13T13:55:55.302Z","caller":"models/shell_command_runner.go:156","msg":"successfully ran \"/atlantis/bin/terraform0.14.7 apply -input=false \\\"/atlantis/repos/bsmith/runatlantis-terraform/12/default/demo/default.tfplan\\\"\" in \"/atlantis/repos/bsmith/runatlantis-terraform/12/default/demo\"","json":{"repo":"bsmith/runatlantis-terraform","pull":"12"}}
{"level":"info","ts":"2022-10-13T13:55:55.302Z","caller":"runtime/apply_step_runner.go:58","msg":"apply successful, deleting planfile","json":{"repo":"bsmith/runatlantis-terraform","pull":"12"}}
{"level":"info","ts":"2022-10-13T13:55:55.386Z","caller":"events/instrumented_project_command_runner.go:53","msg":"apply success. output available at: https://xxx/bsmith/runatlantis-terraform/-/merge_requests/12","json":{"repo":"bsmith/runatlantis-terraform","pull":"12"}}
{"level":"info","ts":"2022-10-13T13:55:55.636Z","caller":"events/automerger.go:32","msg":"automerging pull request","json":{"repo":"bsmith/runatlantis-terraform","pull":"12"}}
{"level":"info","ts":"2022-10-13T13:55:56.269Z","caller":"events/events_controller.go:588","msg":"identified event as type \"closed\"","json":{}}
{"level":"info","ts":"2022-10-13T13:55:56.269Z","caller":"events/instrumented_pull_closed_executor.go:40","msg":"Initiating cleanup of pull data.","json":{"repository":"bsmith/runatlantis-terraform","pull-num":"12"}}
{"level":"info","ts":"2022-10-13T13:55:56.362Z","caller":"events/events_controller.go:461","msg":"deleted locks and workspace for repo bsmith/runatlantis-terraform, pull 12","json":{}}

If not already included, please provide the following:

Atlantis version: atlantis 0.20.1

Could this limitation be caused by the version of GitLab we are running? It appears that required approvals are only available on GitLab Premium and GitLab Ultimate, while we are running Community Edition.
https://docs.gitlab.com/ee/user/project/merge_requests/approvals/index.html

Our company has Gitlab Enterprise and we've noticed a similar issue as well @theautoroboto. Atlantis applies even though there's things that need to be rebased. The (mergeable) apply_requirement is being ignored. We didn't have this issue until recently. It's important to note that we recently updated to a 15.x Gitlab version.

Hello, I have the same problem with apply_requirements that are not evaluated. I am using the aws terraform module which runs Atlantis on fargate (version 3.25.0). I am using GitLab enterprise edition 15.7.0-pre. Did you find solution to solve this problem?

Has this always been an issue with atlantis? Do older versions work as expected? If not, then this may not be a regression and may be related to the gitlab enterprise api change.

Either way, we welcome contributions. If you folks can spot the issue, please feel free to build locally, test it out, and propose a PR.

Hello 👋🏻 I got the same issue on Gitlab SaaS. I tried 2 ways to add my server-side config using the flag --repo-config or --repo-config-json This is not working, I can comment an apply, and it's done without the approval protection.

It's important to notice that I successfully set up an approval protection with the deprecated flag --require-approval But the problem is that this flag fixed the "approved" requirements on plan,apply and import. I only want to configure "approved" on apply

My env : Atlantis latest 0.24.3 GitLab Enterprise 16.2.0 Atlantis installed with the binary on EC2 Permission for Atlantis : group access token with "developer" access

My config : server configuration

atlantis-url: 'https://atlantis.xxxxxx.io/'
gitlab-token: 'xxxxxxxx'
gitlab-user: 'atlantis'
gitlab-webhook-secret: 'xxxxxxxx'
log-level: 'info'
repo-allowlist: 'gitlab.com/xxxx/yyyy/*,gitlab.com/xxxxx/zzzzz/aaaaa/*'
repo-config-json: '{"repos":[{"apply_requirements":["approved"],"id":"/.*/","repo_locking":true}]}'
web-basic-auth: 'true'
web-password: 'xxxxxxxxxxxx'
web-username: 'skello'
write-git-creds: 'true'

service side config

repos:
- id: /.*/
  repo_locking: true
  apply_requirements: [approved]

I tried the deprecated --require-approval flag with Atlantis v0.24.4 and Gitlab v16.4.1 and it seemed to have no effect.

@jukie , I believe you mentioned that you'd volunteer as a Gitlab maintainer, this issue is basically a complete blocker to us adopting Atlantis, do you have some idea on what could be causing this? I'm not familiar with Go or Atlantis' code base at all, but if I get a few pointers I'm willing to dig into what's causing this issue.

@lukemassa do you have any ideas on this? I do not know if you have touched that part of the code

(Note I deleted my earlier comment because it was slightly wrong and confusing)

I did some testing and I learned this has to do with Merge Request Approval Rules (https://docs.gitlab.com/ee/user/project/merge_requests/approvals/rules.html). I'm running atlantis 0.24.4 and gitlab enterprise 16.4.1.

-	`apply_requirements: [approved]`	`apply_requirements: []`
Project has MR Approval Rule	Fails to apply	Applies
Project does not have MR Approval Rule	Applies	Applies

So my current understanding is that you need to have both apply_requirement: [approved] and an approval rule on the project in order for atlantis to refuse to apply an unapproved MR.

By my read, this behavior is contrary to the documentation https://www.runatlantis.io/docs/command-requirements.html#approved. However I'm not sure the best thing to do here. I think the options are:

Update the documentation to say "approved respects the approval rules of the project" (which I think is what's going on)
Update the code to respect the approval rule if it exists, otherwise still require an approval
Update the code to ignore the approval rules and require approval if apply_requirements says approved.

My personal vote is 1 (in fact I've already unknowingly pushed us in that direction: https://github.com/runatlantis/atlantis/pull/3744). But I'd like to understand what other VCSs do. Thoughts @jamengual?

In the meantime, @Angelin01 (or others) can you try setting up a Merge Request Approval Rule (https://docs.gitlab.com/ee/user/project/merge_requests/approvals/rules.html) on your project to see if that blocks unapproved applies? I would expect if both these are set to get an comment back from atlantis that says something like "Apply Failed: Pull request must be approved by at least one person other than the author before running apply."

I agree with @lukemassa here, if you have apply_requirements is becaude you want to have approval rules, therefore, Atlantis should not try to interpret the desired outcome.

I vote for number 1.

@jamengual I've thought about it some more, and I actually think the right solution here is to have approved mean "someone other than the author has approved the PR" across all VCSs, and incorporate "PR meets its approval requirements, whatever they are" as part of mergeable.

This would consist in:

Revert https://github.com/runatlantis/atlantis/pull/3744
Change PullIsApproved logic in for gitlab to be true if there is at least one approval other than author
Move logic for "meets approval requirements" of gitlab into PullIsMergeable
Update documentation to clarify that approved is a consistent notion across VCSs, whereas mergeable is much more complex and deferred to the VCS itself.
See if we can get someone who uses bitbucket or azure to update PullIsMergeable to incorporate approval logic (it's possible they already do I don't know enough about these VCSs to even know what the analogy of an "approval" is).

Looking at Bitbucket and Azure, both attempt to do what github does, which is say a thing is "approved" if someone other than the author has "approved" it, and false otherwise, so I figured it's better to change gitlab to be me more in line with the others, than change the notion of "approved" across the board?

Ahhh that is an interesting point and from the user's point of view it does make sense to think that approved means approved other than the author which is the correct assumption for all VCSs.

I'm all for consistency so I agree with this change, it does make sense.

@GenPage any opinion on this?

I implemented the above in #3830 if you want to take a look. It turns out no change was needed for part 3) since PullIsMergeable already implemented "passing merge request approval rules" for gitlab. Part 5) I'd be happy to look into as a followup, but I don't think that needs to be a blocker here.

runatlantis / atlantis