integrations / terraform-provider-github

Terraform GitHub provider
https://www.terraform.io/docs/providers/github/
MIT License
879 stars 719 forks source link

feat: app-based authentication via env vars without app_auth block #2174

Open laughedelic opened 5 months ago

laughedelic commented 5 months ago

Resolves #1877

This PR adds new provider configuration parameters that mirror those in the app_auth block and make it possible to switch between token-based and app-based authentication via environment variables without altering existing provider configuration code. This allows flexibility of using a GitHub app for provider authentication when running in CI (or another automated environment like Atlantis), and using a personal access token when developing locally.

Related:


Before the change?

After the change?

Example:

provider "github" {
  owner = var.github_owner
}

Pull request checklist

Does this introduce a breaking change?

This was intended as a non-breaking change, so the app_auth block is kept and only new (redundant) parameters are added. Existing behavior is preserved.


Provider auth matrix

Here I want to show different configuration scenarios and outcomes before/after. The only new case is in the first line when the GITHUB_APP_* env vars are set but there's no app_auth block in the code: before it would be an error (app vars would be ignored), but now it works as an app-based configuration.

GITHUB_APP_* GITHUB_TOKEN app_auth {} Before After
❌ error 🤖 app new: no app_auth block needed
✔️ 🔑 token 🔑 token just token auth
🔑 token 🔑 token prioritize token for compatibility
🤖 app 🤖 app prioritize app auth for compatibility
🤖 app 🤖 app app_auth {} is redundant
❌ error ❌ error only app_auth {} with no values

Tests

I'm new to Go, so I need some help to write proper tests for this. I tried manual testing in examples/app_authentication and it worked.

I also added some tests in provider_test.go following the pattern of existing test cases. But in those tests parameters are set explicitly and I'm not sure how to test the behavior of picking up parameters from the environment variables (with an empty provider configuration).

I would love to add tests for all of the cases in the matrix above, but I don't know how to approach it code-wise. Guidance would be highly appreciated 🙏

Code review

The main code change in provider.go nested existing code in an if block, so it's much easier to see the actual change if you review it with whitespace changes ignored.

gulzat214 commented 5 months ago

This would be a good feature to have

nicky-lenaers-phoenicks commented 5 months ago

This would be a very nice feature indeed. Thanks for putting up this PR!