diff --git a/.github/workflows/dotcom-acceptance-tests-all.yml b/.github/workflows/dotcom-acceptance-tests-all.yml index 8828928546..8ba9387ec8 100644 --- a/.github/workflows/dotcom-acceptance-tests-all.yml +++ b/.github/workflows/dotcom-acceptance-tests-all.yml @@ -70,7 +70,7 @@ jobs: with: TF_LOG: INFO RUN_ALL: true - GITHUB_ORGANIZATION: terraformtesting + GITHUB_TEST_ORGANIZATION: terraformtesting GITHUB_TEST_USER_TOKEN: ${{ secrets.DOTCOM_TEST_USER_TOKEN }} GITHUB_TEST_OWNER: github-terraform-test-user @@ -80,6 +80,6 @@ jobs: with: TF_LOG: DEBUG RUN_ALLOWED: ${{ steps.acceptance-tests-organization.outputs.failed }} - GITHUB_ORGANIZATION: terraformtesting + GITHUB_TEST_ORGANIZATION: terraformtesting GITHUB_TEST_USER_TOKEN: ${{ secrets.DOTCOM_TEST_USER_TOKEN }} GITHUB_TEST_OWNER: github-terraform-test-user diff --git a/.github/workflows/dotcom-acceptance-tests.yml b/.github/workflows/dotcom-acceptance-tests.yml index bb2a1be2a9..fdb4a906fe 100644 --- a/.github/workflows/dotcom-acceptance-tests.yml +++ b/.github/workflows/dotcom-acceptance-tests.yml @@ -48,6 +48,6 @@ jobs: uses: terraformtesting/acceptance-tests@v2.2.0 with: TF_LOG: INFO - GITHUB_ORGANIZATION: terraformtesting + GITHUB_TEST_ORGANIZATION: terraformtesting GITHUB_TEST_USER_TOKEN: ${{ secrets.DOTCOM_TEST_USER_TOKEN }} GITHUB_TEST_OWNER: github-terraform-test-user diff --git a/.github/workflows/ghes-acceptance-tests-all.yml b/.github/workflows/ghes-acceptance-tests-all.yml index e889c22f17..9efef5d271 100644 --- a/.github/workflows/ghes-acceptance-tests-all.yml +++ b/.github/workflows/ghes-acceptance-tests-all.yml @@ -86,7 +86,7 @@ jobs: TF_LOG: INFO RUN_ALL: true GITHUB_BASE_URL: "https://terraformtesting-ghe.eastus.cloudapp.azure.com/" - GITHUB_ORGANIZATION: terraformtesting + GITHUB_TEST_ORGANIZATION: terraformtesting GITHUB_TEST_USER_TOKEN: ${{ secrets.GHES_TEST_USER_TOKEN }} - name: Failed Acceptance Tests (Organization) @@ -96,5 +96,5 @@ jobs: TF_LOG: DEBUG RUN_ALLOWED: ${{ steps.acceptance-tests-organization.outputs.failed }} GITHUB_BASE_URL: "https://terraformtesting-ghe.eastus.cloudapp.azure.com/" - GITHUB_ORGANIZATION: terraformtesting + GITHUB_TEST_ORGANIZATION: terraformtesting GITHUB_TEST_USER_TOKEN: ${{ secrets.GHES_TEST_USER_TOKEN }} diff --git a/.github/workflows/ghes-acceptance-tests.yml b/.github/workflows/ghes-acceptance-tests.yml index 2a79df62d6..618e608ce0 100644 --- a/.github/workflows/ghes-acceptance-tests.yml +++ b/.github/workflows/ghes-acceptance-tests.yml @@ -63,5 +63,5 @@ jobs: uses: terraformtesting/acceptance-tests@v2.2.0 with: GITHUB_BASE_URL: "https://terraformtesting-ghe.eastus.cloudapp.azure.com/" - GITHUB_ORGANIZATION: terraformtesting + GITHUB_TEST_ORGANIZATION: terraformtesting GITHUB_TEST_USER_TOKEN: ${{ secrets.GHES_TEST_USER_TOKEN }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 67648975e0..abdadc77ac 100644 --- a/CONTRIBUTING.md +++ b/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= - export GITHUB_ORGANIZATION= + export GITHUB_OWNER= ``` - 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. @@ -176,9 +176,9 @@ Commonly required environment variables are listed below: export TF_LOG=DEBUG # enable testing of organization scenarios instead of individual or anonymous -export GITHUB_ORGANIZATION= +export GITHUB_TEST_ORGANIZATION= -# enable testing of individual scenarios instead of organizaiton or anonymous +# enable testing of individual scenarios instead of organization or anonymous export GITHUB_OWNER= # enable testing of enterprise appliances @@ -193,6 +193,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 individually. + See [this project](https://github.com/terraformtesting/acceptance-tests) for more information on how tests are run automatically. There are also a small amount of unit tests in the provider. Due to the nature of the provider, such tests are currently only recommended for exercising functionality completely internal to the provider. These may be executed by running `make test`. @@ -214,7 +218,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. diff --git a/examples/repository_collaborator/README.md b/examples/repository_collaborator/README.md index 49ff4d3d0e..59e14e8f6a 100644 --- a/examples/repository_collaborator/README.md +++ b/examples/repository_collaborator/README.md @@ -7,7 +7,7 @@ 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= @@ -15,7 +15,7 @@ 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}" diff --git a/github/data_source_github_organization_teams_test.go b/github/data_source_github_organization_teams_test.go index 1fb94910bb..f6398058f2 100644 --- a/github/data_source_github_organization_teams_test.go +++ b/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"), diff --git a/github/data_source_github_repository_test.go b/github/data_source_github_repository_test.go index 7350084293..cc35d419c3 100644 --- a/github/data_source_github_repository_test.go +++ b/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 diff --git a/github/provider.go b/github/provider.go index 0f1beb8d23..e51a6ad653 100644 --- a/github/provider.go +++ b/github/provider.go @@ -3,6 +3,7 @@ package github import ( "fmt" "log" + "os" "strings" "time" @@ -25,13 +26,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, @@ -159,11 +153,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.", @@ -178,31 +168,15 @@ func init() { func providerConfigure(p *schema.Provider) schema.ConfigureFunc { return func(d *schema.ResourceData) (interface{}, error) { owner := d.Get("owner").(string) - baseURL := d.Get("base_url").(string) - 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) + if owner == "" && os.Getenv("GITHUB_OWNER") != "" { + owner = os.Getenv("GITHUB_OWNER") + log.Printf("[DEBUG] Selecting owner %s from GITHUB_OWNER environment variable", owner) } - // END backwards compatibility - org := d.Get("organization").(string) - if org != "" { - log.Printf("[INFO] Selecting organization attribute as owner: %s", org) - owner = org - } + baseURL := d.Get("base_url").(string) + token := d.Get("token").(string) + insecure := d.Get("insecure").(bool) if appAuth, ok := d.Get("app_auth").([]interface{}); ok && len(appAuth) > 0 && appAuth[0] != nil { appAuthAttr := appAuth[0].(map[string]interface{}) diff --git a/github/provider_utils.go b/github/provider_utils.go index 3b52353f6b..cb64fcf2b8 100644 --- a/github/provider_utils.go +++ b/github/provider_utils.go @@ -8,9 +8,10 @@ import ( ) var testCollaborator string = os.Getenv("GITHUB_TEST_COLLABORATOR") -var isEnterprise string = os.Getenv("ENTERPRISE_ACCOUNT") var testOrganization string = testOrganizationFunc() -var testOwner string = os.Getenv("GITHUB_OWNER") + +var isEnterprise string = os.Getenv("ENTERPRISE_ACCOUNT") +var testOwner string = testOwnerFunc() var testToken string = os.Getenv("GITHUB_TOKEN") var testBaseURLGHES string = os.Getenv("GHES_BASE_URL") @@ -18,8 +19,11 @@ 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_ORGANIZATION"); v == "" { + t.Fatal("GITHUB_TEST_ORGANIZATION 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") @@ -43,23 +47,29 @@ func skipUnlessMode(t *testing.T, providerMode string) { 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: 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") + t.Logf("GITHUB_TOKEN and GITHUB_OWNER environment variables should be set for tests in %v mode", providerMode) } case organization: - if os.Getenv("GITHUB_TOKEN") != "" && os.Getenv("GITHUB_ORGANIZATION") != "" { + testOwner := os.Getenv("GITHUB_TEST_ORGANIZATION") + if testOwner == "" { + testOwner = os.Getenv("GITHUB_OWNER") + } + if os.Getenv("GITHUB_TOKEN") != "" && testOwner != "" { + log.Printf("[DEBUG] configuring user type: %s", providerMode) return } else { - t.Log("GITHUB_TOKEN and GITHUB_ORGANIZATION environment variables should be set") + t.Logf("GITHUB_TOKEN and GITHUB_TEST_ORGANIZATION environment variables should be set for tests in %v mode", providerMode) } } @@ -71,13 +81,11 @@ func testAccCheckOrganization() error { baseURL := os.Getenv("GITHUB_BASE_URL") token := os.Getenv("GITHUB_TOKEN") - 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 + owner := os.Getenv("GITHUB_TEST_ORGANIZATION") + if owner == "" && os.Getenv("GITHUB_OWNER") == "" { + return fmt.Errorf("neither `GITHUB_TEST_ORGANIZATION` nor `GITHUB_OWNER` are set in environment") + } else if owner == "" { + owner = os.Getenv("GITHUB_OWNER") } config := Config{ @@ -96,32 +104,26 @@ func testAccCheckOrganization() error { return nil } -func OwnerOrOrgEnvDefaultFunc() (interface{}, error) { - if organization := os.Getenv("GITHUB_ORGANIZATION"); organization != "" { - log.Printf("[INFO] Selecting owner %s from GITHUB_ORGANIZATION environment variable", organization) - return organization, nil - } +func testOwnerFunc() string { owner := os.Getenv("GITHUB_OWNER") - log.Printf("[INFO] 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: +// GITHUB_ORGANIZATION is deprecated. The purpose of this variable is +// 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" diff --git a/website/docs/index.html.markdown b/website/docs/index.html.markdown index bd6d363c24..86f482e0e7 100644 --- a/website/docs/index.html.markdown +++ b/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.