Auth changes for vNext

CONTRIBUTING.md

@@ -38,7 +38,7 @@ Once you have the repository cloned, there's a couple of additional steps you'll
 - Export the necessary configuration for authenticating your provider with GitHub
   ```sh
   export GITHUB_TOKEN=<token of a user with an organization account>
-  export GITHUB_ORGANIZATION=<name of an organization>
+  export GITHUB_OWNER=<name of an organization>
   ```
 - Build the project with `make build`
 - Try an example test run from the default (`master`) branch, like `TF_LOG=DEBUG TF_ACC=1 go test -v   ./... -run ^TestAccGithubRepositories`. All those tests should pass.
@@ -199,6 +199,10 @@ export GITHUB_TEST_ORGANIZATION=
 export GITHUB_TEST_USER_TOKEN=
 ```
 
+Note that the above `GITHUB_TEST_ORGANIZATION` variable does not have a corresponding `GITHUB_ORGANIZATION` environment variable for use outside of tests. The `GITHUB_ORGANIZATION` variable was deprecated in version 5.0.0 of the GitHub Terraform provider, and the sole way to configure an organization is to use the `owner` provider block configuration or the `GITHUB_OWNER` variable.
+
+Use the `GITHUB_TEST_ORGANIZATION` variable only in acceptance tests, only to specify an organization owner. This is useful in provider scenarios that are valid for both individual and organization owners and need to be tested appropriately.
+
 See [this project](https://github.com/terraformtesting/acceptance-tests) for more information on how tests are run automatically.
 
 ### GitHub Personal Access Token
@@ -218,7 +222,7 @@ Once the token has been created, it must be exported in your environment as `GIT
 
 ### GitHub Organization
 
-If you do not have an organization already that you are comfortable running tests against, you will need to [create one](https://help.github.com/en/articles/creating-a-new-organization-from-scratch). The free "Team for Open Source" org type is fine for these tests. The name of the organization must then be exported in your environment as `GITHUB_ORGANIZATION`.
+If you do not have an organization already that you are comfortable running tests against, you will need to [create one](https://help.github.com/en/articles/creating-a-new-organization-from-scratch). The free "Team for Open Source" org type is fine for these tests. The name of the organization must then be exported in your environment as `GITHUB_OWNER`.
 
 Make sure that your organization has a `terraform-module-template` repository ([terraformtesting/terraform-template-module](https://github.com/terraformtesting/terraform-template-module) is an example you can clone) and that its "Template repository" item in Settings is checked.
 

examples/repository_collaborator/README.md

@@ -7,15 +7,15 @@ This example will also create a repository in the specified `owner` organization
 Alternatively, you may use variables passed via command line:
 
 ```console
-export GITHUB_ORGANIZATION=
+export GITHUB_OWNER=
 export GITHUB_TOKEN=
 export COLLABORATOR_USERNAME=
 export COLLABORATOR_PERMISSION=
 ```
 
 ```console
 terraform apply \
-  -var "organization=${GITHUB_ORGANIZATION}" \
+  -var "owner=${GITHUB_OWNER}" \
   -var "github_token=${GITHUB_TOKEN}" \
   -var "username=${COLLABORATOR_USERNAME}" \
   -var "permission=${COLLABORATOR_PERMISSION}"

github/data_source_github_organization_teams_test.go

@@ -1,7 +1,6 @@
 package github
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
@@ -11,9 +10,9 @@ func TestAccGithubOrganizationTeamsDataSource(t *testing.T) {
 
 	t.Run("queries without error", func(t *testing.T) {
 
-		config := fmt.Sprintf(`
+		config := `
 			data "github_organization_teams" "all" {}
-		`)
+		`
 
 		check := resource.ComposeAggregateTestCheckFunc(
 			resource.TestCheckResourceAttrSet("data.github_organization_teams.all", "teams.0.id"),
@@ -49,11 +48,11 @@ func TestAccGithubOrganizationTeamsDataSource(t *testing.T) {
 
 	t.Run("queries root teams only without error", func(t *testing.T) {
 
-		config := fmt.Sprintf(`
+		config := `
 			data "github_organization_teams" "root_teams" {
 				root_teams_only = true
 			}
-		`)
+		`
 
 		check := resource.ComposeAggregateTestCheckFunc(
 			resource.TestCheckResourceAttrSet("data.github_organization_teams.root_teams", "teams.0.id"),

github/data_source_github_repository_test.go

@@ -101,7 +101,7 @@ func TestAccGithubRepositoryDataSource(t *testing.T) {
 		randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
 
 		config := fmt.Sprintf(`
-		
+
 			resource "github_repository" "test" {
 				name         = "tf-acc-%s"
 				auto_init    = true

github/provider.go

@@ -25,13 +25,6 @@ func Provider() terraform.ResourceProvider {
 				DefaultFunc: schema.EnvDefaultFunc("GITHUB_OWNER", nil),
 				Description: descriptions["owner"],
 			},
-			"organization": {
-				Type:        schema.TypeString,
-				Optional:    true,
-				DefaultFunc: schema.EnvDefaultFunc("GITHUB_ORGANIZATION", nil),
-				Description: descriptions["organization"],
-				Deprecated:  "Use owner (or GITHUB_OWNER) instead of organization (or GITHUB_ORGANIZATION)",
-			},
 			"base_url": {
 				Type:        schema.TypeString,
 				Optional:    true,
@@ -158,11 +151,7 @@ func init() {
 
 		"insecure": "Enable `insecure` mode for testing purposes",
 
-		"owner": "The GitHub owner name to manage. " +
-			"Use this field instead of `organization` when managing individual accounts.",
-
-		"organization": "The GitHub organization name to manage. " +
-			"Use this field instead of `owner` when managing organization accounts.",
+		"owner": "The GitHub organization or user name to manage. ",
 
 		"app_auth": "The GitHub App credentials used to connect to GitHub. Conflicts with " +
 			"`token`. Anonymous mode is enabled if both `token` and `app_auth` are not set.",
@@ -181,28 +170,6 @@ func providerConfigure(p *schema.Provider) schema.ConfigureFunc {
 		token := d.Get("token").(string)
 		insecure := d.Get("insecure").(bool)
 
-		// BEGIN backwards compatibility
-		// OwnerOrOrgEnvDefaultFunc used to be the default value for both
-		// 'owner' and 'organization'. This meant that if 'owner' and
-		// 'GITHUB_OWNER' were set, 'GITHUB_OWNER' would be used as the default
-		// value of 'organization' and therefore override 'owner'.
-		//
-		// This seems undesirable (an environment variable should not override
-		// an explicitly set value in a provider block), but is necessary
-		// for backwards compatibility. We could remove this backwards compatibility
-		// code in a future major release.
-		env, _ := OwnerOrOrgEnvDefaultFunc()
-		if env.(string) != "" {
-			owner = env.(string)
-		}
-		// END backwards compatibility
-
-		org := d.Get("organization").(string)
-		if org != "" {
-			log.Printf("[DEBUG] Selecting organization attribute as owner: %s", org)
-			owner = org
-		}
-
 		if appAuth, ok := d.Get("app_auth").([]interface{}); ok && len(appAuth) > 0 && appAuth[0] != nil {
 			appAuthAttr := appAuth[0].(map[string]interface{})
 

github/provider_utils.go

@@ -8,8 +8,9 @@ import (
 )
 
 var testCollaborator string = os.Getenv("GITHUB_TEST_COLLABORATOR")
-var isEnterprise string = os.Getenv("ENTERPRISE_ACCOUNT")
 var testOrganization string = testOrganizationFunc()
+
+var isEnterprise string = os.Getenv("ENTERPRISE_ACCOUNT")
 var testOwner string = os.Getenv("GITHUB_OWNER")
 var testToken string = os.Getenv("GITHUB_TOKEN")
 var testBaseURLGHES string = os.Getenv("GHES_BASE_URL")
@@ -18,8 +19,8 @@ func testAccPreCheck(t *testing.T) {
 	if v := os.Getenv("GITHUB_TOKEN"); v == "" {
 		t.Fatal("GITHUB_TOKEN must be set for acceptance tests")
 	}
-	if v := os.Getenv("GITHUB_ORGANIZATION"); v == "" && os.Getenv("GITHUB_OWNER") == "" {
-		t.Fatal("GITHUB_ORGANIZATION or GITHUB_OWNER must be set for acceptance tests")
+	if v := os.Getenv("GITHUB_OWNER"); v == "" {
+		t.Fatal("GITHUB_OWNER must be set for acceptance tests")
 	}
 	if v := os.Getenv("GITHUB_TEST_USER"); v == "" {
 		t.Fatal("GITHUB_TEST_USER must be set for acceptance tests")
@@ -36,33 +37,41 @@ func testAccPreCheck(t *testing.T) {
 }
 
 func skipUnlessMode(t *testing.T, providerMode string) {
+	log.Printf("[DEBUG] <<<<<<< skipUnlessMode")
 	switch providerMode {
 	case anonymous:
+		log.Printf("[DEBUG] <<<<<<< anonymous")
 		if os.Getenv("GITHUB_BASE_URL") != "" &&
 			os.Getenv("GITHUB_BASE_URL") != "https://api.github.com/" {
 			t.Log("anonymous mode not supported for GHES deployments")
 			break
 		}
 
 		if os.Getenv("GITHUB_TOKEN") == "" {
+			log.Printf("[DEBUG] <<<<<<< configuring anonymous user without token")
 			return
 		} else {
-			t.Log("GITHUB_TOKEN environment variable should be empty")
+			t.Log("GITHUB_TOKEN environment variable should be empty in anonymous mode")
 		}
-	case individual:
+	case individual, organization:
+		log.Printf("[DEBUG] <<<<<<< user type: %s", providerMode)
 		if os.Getenv("GITHUB_TOKEN") != "" && os.Getenv("GITHUB_OWNER") != "" {
+			log.Printf("[DEBUG] <<<<<<< configuring user type: %s", providerMode)
 			return
 		} else {
-			t.Log("GITHUB_TOKEN and GITHUB_OWNER environment variables should be set")
-		}
-	case organization:
-		if os.Getenv("GITHUB_TOKEN") != "" && os.Getenv("GITHUB_ORGANIZATION") != "" {
-			return
-		} else {
-			t.Log("GITHUB_TOKEN and GITHUB_ORGANIZATION environment variables should be set")
+			t.Logf("GITHUB_TOKEN and GITHUB_OWNER environment variables should be set for tests in %v mode", providerMode)
 		}
+		// TODO(kfcampbell): this is a problem. how are we going to know it's an organization
+		// in acceptance tests?
+		// ideas:
+		// - we could perform a lookup online to determine the type
+		// - we could use the GITHUB_ORG variable only for acceptance tests
+		// - we could switch GITHUB_ORG to be a boolean
+		// - we could deprecate GITHUB_ORG entirely and create
+		// 	another environment variable boolean to use instead
 	}
 
+	log.Printf("[DEBUG] <<<<<<< skipping!")
 	t.Skipf("Skipping %s which requires %s mode", t.Name(), providerMode)
 }
 
@@ -73,11 +82,7 @@ func testAccCheckOrganization() error {
 
 	owner := os.Getenv("GITHUB_OWNER")
 	if owner == "" {
-		organization := os.Getenv("GITHUB_ORGANIZATION")
-		if organization == "" {
-			return fmt.Errorf("Neither `GITHUB_OWNER` or `GITHUB_ORGANIZATION` set in environment")
-		}
-		owner = organization
+		return fmt.Errorf("`GITHUB_OWNER` not set in environment")
 	}
 
 	config := Config{
@@ -96,32 +101,26 @@ func testAccCheckOrganization() error {
 	return nil
 }
 
-func OwnerOrOrgEnvDefaultFunc() (interface{}, error) {
-	if organization := os.Getenv("GITHUB_ORGANIZATION"); organization != "" {
-		log.Printf("[DEBUG] Selecting owner %s from GITHUB_ORGANIZATION environment variable", organization)
-		return organization, nil
-	}
+func testOwnerFunc() string {
 	owner := os.Getenv("GITHUB_OWNER")
-	log.Printf("[DEBUG] Selecting owner %s from GITHUB_OWNER environment variable", owner)
-	return owner, nil
+	if owner == "" {
+		owner = os.Getenv("GITHUB_TEST_OWNER")
+	}
+	return owner
 }
 
+// testOrganizationFunc returns a test organization. IMPORTANT:
+// since GITHUB_ORGANIZATION is deprecated, this is an ugly fallback
+// to make sure that we can still test organization cases appropriately
+// in integration testing.
 func testOrganizationFunc() string {
-	organization := os.Getenv("GITHUB_ORGANIZATION")
+	organization := os.Getenv("GITHUB_TEST_ORGANIZATION")
 	if organization == "" {
-		organization = os.Getenv("GITHUB_TEST_ORGANIZATION")
+		organization = os.Getenv("GITHUB_OWNER")
 	}
 	return organization
 }
 
-func testOwnerFunc() string {
-	owner := os.Getenv("GITHUB_OWNER")
-	if owner == "" {
-		owner = os.Getenv("GITHUB_TEST_OWNER")
-	}
-	return owner
-}
-
 const anonymous = "anonymous"
 const individual = "individual"
 const organization = "organization"

website/docs/index.html.markdown

@@ -98,9 +98,7 @@ The following arguments are supported in the `provider` block:
 
 * `base_url` - (Optional) This is the target GitHub base API endpoint. Providing a value is a requirement when working with GitHub Enterprise. It is optional to provide this value and it can also be sourced from the `GITHUB_BASE_URL` environment variable. The value must end with a slash, for example: `https://terraformtesting-ghe.westus.cloudapp.azure.com/`
 
-* `owner` - (Optional) This is the target GitHub organization or individual user account to manage. For example, `torvalds` and `github` are valid owners. It is optional to provide this value and it can also be sourced from the `GITHUB_OWNER` environment variable. When not provided and a `token` is available, the individual user account owning the `token` will be used. When not provided and no `token` is available, the provider may not function correctly.
-
-* `organization` - (Deprecated) This behaves the same as `owner`, which should be used instead. This value can also be sourced from the `GITHUB_ORGANIZATION` environment variable.
+* `owner` - (Optional) This is the target GitHub organization or individual user account to manage. For example, `torvalds` and `github` are valid owners. It is optional to provide this value and it can also be sourced from the `GITHUB_OWNER` environment variable. When not provided and a `token` is available, the individual user account owning the `token` will be used. When not provided and no `token` is available, the provider may not function correctly. If both `owner` and `GITHUB_OWNER` are set, `owner` takes priority.
 
 * `app_auth` - (Optional) Configuration block to use GitHub App installation token. When not provided, the provider can only access resources available anonymously.
   * `id` - (Required) This is the ID of the GitHub App. It can sourced from the `GITHUB_APP_ID` environment variable.
@@ -111,15 +109,8 @@ The following arguments are supported in the `provider` block:
 
 Note: If you have a PEM file on disk, you can pass it in via `pem_file = file("path/to/file.pem")`.
 
-For backwards compatibility, if more than one of `owner`, `organization`,
-`GITHUB_OWNER` and `GITHUB_ORGANIZATION` are set, the first in this
-list takes priority.
+## Authenticating with multiple providers
 
-1. Setting `organization` in the GitHub provider configuration.
-2. Setting the `GITHUB_ORGANIZATION` environment variable.
-3. Setting the `GITHUB_OWNER` environment variable.
-4. Setting `owner` in the GitHub provider configuration.
+When authenticating with multiple providers, it is possible to fail to authenticate both of them correctly and experience unexpected behavior. This is especially fraught when using both `integrations/github` and `hashicorp/github`. As [this GitHub comment](https://github.com/integrations/terraform-provider-github/issues/655#issuecomment-804419698) from @tibbes details, [local names](https://www.terraform.io/docs/language/providers/requirements.html#local-names) must be used when using two providers with the same name. Otherwise, as @tibbes mentions, the `provider "github"` block is sent to one of the providers but not the other, which means that the other provider is working in anonymous mode with no token, no owner, and no organization.
 
-~> It is a bug that `GITHUB_OWNER` takes precedence over `owner`, which may
-be fixed in a future major release. For compatibility with future releases,
-please set only one of `GITHUB_OWNER` and `owner`.
+If you are experiencing unexpected behavior with the provider and your HCL config contains both `integrations/github` and `hashicorp/github`, consider using only one of the providers or else local names in your configuration.