feat: Added support for GitHub app

[abalcobia]

Description

Allow users to utilize the GitHub App instead of a personal access token to execute Atlantis.

Motivation and Context

Enable the use of a GitHub App with Atlantis. In this PR, four variables were added to the module and their dependencies:

• var.atlantis_github_app_key
• var.atlantis_github_app_key_ssm_parameter_name
• var.atlantis_write_git_creds
• var.atlantis_github_app_id

Note: It is not necessary to create a specific variable for the webhook secret in the GitHub App, the existing one can be used.

• fixes #271

ref:https://www.runatlantis.io/docs/access-credentials.html#generating-an-access-token (GitHub App section)

Breaking Changes

None

How Has This Been Tested?

• [x] I have executed pre-commit run -a on my pull request

Initially, a github app was created and consequently a private key and a secret webhook were associated with it. Posteriorly, a test repo was used with the inputs (atlantis_github_app_id, atlantis_github_app_key & atlantis_github_webhook_secret) from the github app to guarantee the communication between Atlantis and the GitHub app at the level of webhooks and comments and it was possible to guarantee this communication with success as well as the provisioning of resources.

[bryantbiggs]

Thanks for the PR @abalcobia - since GitHub apps are more secure and the recommended way to provide access, lets update the github-complete to use this functionality and provide any docs that users would need to get this setup to use or validate

[abalcobia]

Regarding the updates and the docs on examples, I will work on that in the next days. Thanks!

[calexandre]

@bryantbiggs @antonbabenko I've helped @abalcobia with the latest changes. Can you guys review please?

[calexandre]

Thanks for the review @bryantbiggs! we'll make the requested changes asap 👍

[abalcobia]

Hello @bryantbiggs! Just a question to clarify the approach, is it only supposed to remove the user and token variables, and respective dependencies, in the folder examples or throughout the entire repo?

[bryantbiggs]

@abalcobia - correct, just remove those from the example. It doesn't make a lot of sense to have two authentication routes for one example, I don't even know if this works, so lets just show the "preferred" way which is to use the GitHub app authentication route in the example. Does that make sense? The user/token authentication route will still exist in the module itself, we're just changing the example to show using the GitHub app route.

[abalcobia]

@bryantbiggs - yes, makes sense, thanks. We get the idea and throughout the day we will update the README.md file only in the examples folder.

[calexandre]

@bryantbiggs we've included instructions on how to generate a github app. We've also added a note stating that using GitHub PATs is no longer recommended (despite being still supported).

Let us know if you need any more changes.

[calexandre]

@bryantbiggs we've updated the PR with the latest changes from the master.

Can we get this merged on the master, or is something missing?

[abalcobia]

Hello @bryantbiggs @antonbabenko

If you feel that any more changes need to be made please let us know.

Thanks.

[bryantbiggs]

I will test this today

[calexandre]

@bryantbiggs sorry to bother, but do you have any updates on this? We've updated the branch to the latest commit on master...

[github-actions[bot]]

This PR has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 10 days

[matthiassb]

Commenting to keep open

[calexandre]

rebased and squashed the commit history

[bryantbiggs]

apologies for the delay - just getting back around to this. I'm a bit confused: what is the benefit here for GitHub app support if GitHub PAT is still required?

[calexandre]

Well, it depends if you want to keep supporting PAT based installations... Do you want us to remove all PAT references?

[bryantbiggs]

I don't think thats the way to look at it - there are two avenues:

1. User elects to use GitHub PAT (and only PAT)
2. User elects to use GitHub App (and only App - no PAT)

I believe both can be supported for now to avoid a breaking change, but this current implementation does not satisfy point 2 above which makes the current implementation a bit confusing. Why use a GitHub app if a PAT is still required?

[calexandre]

I don't think thats the way to look at it - there are two avenues:

1. User elects to use GitHub PAT (and only PAT)
2. User elects to use GitHub App (and only App - no PAT)

I believe both can be supported for now to avoid a breaking change, but this current implementation does not satisfy point 2 above which makes the current implementation a bit confusing. Why use a GitHub app if a PAT is still required?

I'm confused...All PAT related variables are optional.. Can you please request a change on where the current implementation does not satisfy the requirements? Maybe it is easier to understand what you mean...

[bryantbiggs]

@calexandre simply try deploying the github-complete example, it does not work as defined in this PR currently. You can see why it won't work because a PAT is still required for the webhook https://github.com/terraform-aws-modules/terraform-aws-atlantis/blob/master/modules/github-repository-webhook/main.tf#L2-L4

[calexandre]

Oh, that must have slipped on a previous rebase... I will look into that in a moment!

Thank you for pointing it out 👍

[calexandre]

Just note that it is not possible to configure the github provider using both approaches, you either need to choose the PAT of the APP approach, or use two providers...

[bryantbiggs]

Just note that it is not possible to configure the github provider using both approaches, you either need to choose the PAT of the APP approach, or use two providers...

Is it possible to set the variables to null as default - I was going to try this but the example wasn't working to start so didn't get to it. Might have to be part of a larger refactor and externalize the provider like usual

[calexandre]

@bryantbiggs I re-added the github_token variable on all modules that require github-repository-webhook to make this work for both scenarios.

Please note that only Atlantis should benefit from the github app auth method as it is more suited for this type applications.

It is debatable whether the github-repository-webhook should use PAT or APP authentication; regardless of which method is preferred I believe it is outside of the scope of this PR, since it would have to be a different APP with a different set of permissions.

Note that Atlantis default/recommended application permissions don't involve any kind of repository settings interaction, which would require changes to be able to create webhooks.

[calexandre]

Just note that it is not possible to configure the github provider using both approaches, you either need to choose the PAT of the APP approach, or use two providers...

Is it possible to set the variables to null as default - I was going to try this but the example wasn't working to start so didn't get to it. Might have to be part of a larger refactor and externalize the provider like usual

I was not able to setup the github provider to work with both methods in the same provider. The only way is to setup a second provider alias and use the APP auth on that one.

You are right, the correct way would be to take the provider out of the module and let the invoker configure the provider.

BUT still, I do believe that the webhook creation is not related to how Atlantis authenticates itself against GitHub.

[calexandre]

@bryantbiggs I also forgot to mention that with a GitHub App you no longer need to create a repository webhook for sending events... That part is configured on the GitHub App settings.

One suggestion to make the example a bit more elegant could be to conditionally invoke the webhook module based on the presence of app auth variables: github_app_id and github_app_key respectively.

[bryantbiggs]

if thats the case then lets remove the webhook from the example. the webhook is only there to send the payload to Atlantis when there are changes

[bryantbiggs]

(my bad - wrong button 🤣 )

[calexandre]

if thats the case then lets remove the webhook from the example. the webhook is only there to send the payload to Atlantis when there are changes

In that case I will remove all variables and references to the github-repository-webhook module (including the module itself). Is that okay?

[calexandre]

if thats the case then lets remove the webhook from the example. the webhook is only there to send the payload to Atlantis when there are changes

In that case I will remove all variables and references to the github-repository-webhook module (including the module itself). Is that okay?

@bryantbiggs could you please give more details on what you mean by removing the webhook from the example? You meant to remove all the webhook implementations, or just from the particular github-complete example while keeping the module, etc..?

On the other hand, we could also copy the existing example and name it to something like github-complete-app-auth and remove the webhook references from there. That way you keep both examples and preserve retrocompatibility.

Just let me know how you want to proceed so that we can get this merged :)