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

[laughedelic]

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:

• [FEAT]: Support app auth and PAT via two provider instances #1537 should still work as described (since app_auth is not going anywhere)

---

Before the change?

• app_auth {} block is required for app-based provider authentication
• it's not possible to switch between token-based and app-based authentication without modifying provider configuration code

After the change?

• it's possible to use app-based authentication by setting GITHUB_APP_* env vars without an app_auth block in the code
• same code can be used in different environments either with GITHUB_TOKEN or GITHUB_APP_* env vars

Example:

provider "github" {
  owner = var.github_owner
}

• if GITHUB_TOKEN is set, provider will pick it up
• otherwise, if GITHUB_APP_* are set, provider will use app-based auth and generate an app installation token

Pull request checklist

• Tests for the changes have been added (for bug fixes / features)
  • Some tests added, but need more work. See below for details ⬇️
• Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

• No

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.

[laughedelic]

I didn't have luck running those tests locally:

$ GITHUB_APP_ID='xxx' GITHUB_APP_INSTALLATION_ID='xxx' GITHUB_APP_PEM_FILE='xxx' GITHUB_OWNER='xxx' TF_ACC=1 go test -v ./... -run ^TestAccProviderConfigure

...
=== RUN   TestAccProviderConfigure/can_be_configured_to_run_anonymously
    provider_test.go:60: Step 1/1 error: Error running pre-apply refresh: exit status 1
        
        Error: Duplicate provider configuration
        
          on terraform_plugin_test.tf line 3:
           3:                   provider "github" {}
        
        A default (non-aliased) provider configuration for "github" was already given
        at terraform_plugin_test.tf:1,1-18. If multiple configurations are required,
        set the "alias" argument for alternative configurations.

I don't quite understand where this is coming from and how to fix it.

[kfcampbell]

It's complaining because on line 111 below, you're already setting a provider, which is the provider defined here.

If it makes you feel better, these tests are broken as-is, it's not you. I'm not sure if they ever even worked.

I'm looking at the Azure Terraform provider tests as an example, and while they do have some configuration to tweak the provider, it's about tweaking the schema, not what's provided to it.

I'm not sure what the best way of validating this behavior would be. Hashicorp plugin development testing documentation doesn't appear to address this case. Probably in a best case scenario, we'd add another Actions workflow with a separate configuration to run tests as a whole. Thoughts?

[laughedelic]

Thanks for the pointers!

these tests are broken as-is, it's not you. I'm not sure if they ever even worked

I see that all acceptance tests are failing in CI. Do these tests run at all? Or are they just ignored?

Reading the docs, I think the Config field in the TestSteps is misused for the provider configuration (which as you pointed out is already initialized). Maybe it's possible to refactor this, but I wouldn't brave it (at least not in the scope of this PR).

Probably in a best case scenario, we'd add another Actions workflow with a separate configuration to run tests as a whole. Thoughts?

I see that in the current CI setup there is a job per authentication method: anonymous, individual and organization. Are you suggesting adding another job for the app-based authentication? I think it would generally make sense. But that would require fixing current issues with CI and setting up an app for testing.

[gulzat214]

This would be a good feature to have

[nicky-lenaers-phoenicks]

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