server-side atlantis.yaml

[atlantisbot]

Issue by @so0k
Monday Jan 29, 2018 at 08:24 GMT
Migrated from hootsuite/atlantis#236
Why was it migrated?

---

Atm, any developer can add pre_get, pre_init, pre/post_plan commands to expose secrets via atlantis.yaml - as Atlantis requires quite significant permissions, this might be a concern.

Drone used to have a signing step required for the drone.yaml file, ensuring unauthorized modifications to atlantis.yaml can be prevented

[atlantisbot]

Comment by @so0k
Monday Jan 29, 2018 at 08:46 GMT

---

My mistake, output of these hooks isn't printed to comments

[atlantisbot]

Comment by @lkysow
Monday Jan 29, 2018 at 20:50 GMT

---

I think this is still an issue actually. If there's an error while running the scripts then we print out the output if you run the command with --verbose.

[atlantisbot]

Comment by @lkysow
Thursday Feb 08, 2018 at 21:19 GMT

---

I think the best way to fix this is to only execute the atlantis.yaml file that's on the branch being pull requested into.

[atlantisbot]

Comment by @eriksw
Friday Feb 09, 2018 at 00:04 GMT

---

Using old atlantis.yaml seems problematic. The most trivial example that springs to mind is a PR that changes what version of terraform is being used, with simultaneous changes to atlantis.yaml and .tf changes that require the new version of terraform.

[atlantisbot]

Comment by @lkysow
Friday Feb 09, 2018 at 00:18 GMT

---

@eriksw in that case you'd have to do 2 pull requests. The first would be the version change and the second would be the tf changes.

It's kind of annoying but if we use the atlantis.yaml on the pull request it seems like we open ourselves up to lots of issues that circumvent the code review and authorization aspect of Atlantis, like the example the issue @so0k mentioned.

I anticipate that atlantis.yaml won't be changing too much. Basically you set up your repo and then you're good to go. So it might be okay.

[atlantisbot]

Comment by @eriksw
Friday Feb 09, 2018 at 01:25 GMT

---

I can't say I'm a fan of having to go through a two-PR process every time there's a new version of terraform. (Pinning in both .tf terraform { .. } and atlantis.yaml.)

The simplicity of requiring a simple detached hmac "signature" of atlantis.yaml is pretty appealing in comparison.

[atlantisbot]

Comment by @lkysow
Sunday Feb 11, 2018 at 02:20 GMT

---

@eriksw you're right that that is a poor workflow. Thank you for your concerns. As a result I've spent some more time looking into how drone.io deals with these issues and why they removed the signing step. This thread is particularly illuminating: harness/harness#1935

My conclusions:

• Requiring signing atlantis.yaml would be a poor user experience for the reasons experienced by drone. It would also require some changes I don't like very much. Either starting atlantis server with a secret signing key option, ex. -signing-key=<secret> that then everyone needs to know in order to sign the atlantis.yaml or having some sort of login system into Atlantis which is more than I think the project should bite off at this moment.
• Only executing the atlantis.yaml from the base branch (requiring a pull request merge to make changes) is also a poor user experience for the reasons Erik mentioned and also for testing new files.
• Thus I'm leaning more towards the solution that drone settled on: if we detect a change in atlantis.yaml we won't run any commands unless the user doing the pull request is in the list of admins for the project OR the PR is approved. In a later change, we will need to implement something like a gatekeeper endpoint (see drone's here: RFC improved gating capabilities, whitelists harness/harness#1971 (comment)) or hook into an identity system to make this more configurable.

[atlantisbot]

Comment by @so0k
Tuesday Feb 13, 2018 at 01:15 GMT

---

I like the conclusion - in Drone we do provide a list of admins as part of the configuration, but you will get this from the repo attributes?

Does this mean atlantis plan won't work for PRs that include changes to atlantis.yaml unless the PR is "trusted" - where trusted means the user creating the PR is an admin collaborator or belongs to a team that have admin rights?

[atlantisbot]

Comment by @lkysow
Tuesday Feb 13, 2018 at 20:42 GMT

---

but you will get this from the repo attributes

Yes, for now until I have time to work on a more robust authentication/authorization system

Does this mean atlantis plan won't work for PRs that include changes to atlantis.yaml unless the
PR is "trusted"

Yes, exactly–because otherwise you could put malicious changes in the plan step

[atlantisbot]

Comment by @grobie
Tuesday Feb 27, 2018 at 16:50 GMT

---

Another use case:

• mono repo with hundreds of commits per hour
• pushing to master directly can't be forbidden

The proposed solution there won't work, as a user could simply change atlantis.yml in master and then create a PR. An additionally solution for such mono repo use cases: separate atlantis.yml from the terraform files and load it directly on the (secured) server.

[lkysow]

I've created an RFC to address this change with a proposed schema. Please add any comments there.

[nikovirtala]

+1 for the server-side config described in the RFC. It would allow us to enforce controls and implement custom workflows centrally, which is well... needed in enterprise environments like ours, where you have numerous teams, multiple vendors, and a wide variety in Terraform skills level.

[boggsboggs]

Also +1 on the server-side config described in the RFC. Seems like a good solution to me and is clearly needed.

[lkysow]

Hi All,
If you're interested in trying out server-side config, I've created an alpha release. You can get it here: https://github.com/runatlantis/atlantis/releases/tag/v0.7.0-alpha1 or from Docker: runatlantis/atlantis:v0.7.0-alpha.

• You can read docs about it here: https://deploy-preview-546--runatlantis.netlify.com/docs/repos-yaml-reference.html#overview
• If you're wondering about what this feature is, read: https://docs.google.com/document/d/1w2wePfXGA3L151Af6D0kB1aKVB7pXys_l9Vk9SD7kyA/edit#
• If you've got feedback, either reply here or open up an issue. Thanks!

[lkysow]

Closed by release v0.7.0