From c2f5793a2cf7029cd23a33ce278fb57831b1cdca Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 2 Jan 2026 00:15:27 +0200 Subject: [PATCH 1/9] Ensure that `token` is never configured together with `app_auth` Signed-off-by: Timo Sand --- github/provider.go | 18 ++++++++++-------- github/provider_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/github/provider.go b/github/provider.go index d96b919522..a1a542e103 100644 --- a/github/provider.go +++ b/github/provider.go @@ -18,10 +18,11 @@ func Provider() *schema.Provider { p := &schema.Provider{ Schema: map[string]*schema.Schema{ "token": { - Type: schema.TypeString, - Optional: true, - DefaultFunc: schema.EnvDefaultFunc("GITHUB_TOKEN", nil), - Description: descriptions["token"], + Type: schema.TypeString, + Optional: true, + DefaultFunc: schema.EnvDefaultFunc("GITHUB_TOKEN", nil), + Description: descriptions["token"], + ConflictsWith: []string{"app_auth"}, }, "owner": { Type: schema.TypeString, @@ -93,10 +94,11 @@ func Provider() *schema.Provider { Description: descriptions["parallel_requests"], }, "app_auth": { - Type: schema.TypeList, - Optional: true, - MaxItems: 1, - Description: descriptions["app_auth"], + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Description: descriptions["app_auth"], + ConflictsWith: []string{"token"}, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "id": { diff --git a/github/provider_test.go b/github/provider_test.go index 5a7915cab7..5387edcb23 100644 --- a/github/provider_test.go +++ b/github/provider_test.go @@ -2,6 +2,7 @@ package github import ( "fmt" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -214,4 +215,30 @@ func TestAccProviderConfigure(t *testing.T) { }, }) }) + t.Run("should not allow both token and app_auth to be configured", func(t *testing.T) { + config := fmt.Sprintf(` + provider "github" { + owner = "%s" + token = "%s" + app_auth { + id = "1234567890" + installation_id = "1234567890" + pem_file = "1234567890" + } + } + + data "github_ip_ranges" "test" {} + `, testAccConf.owner, testAccConf.token) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("\"app_auth\": conflicts with token"), + }, + }, + }) + }) } From 5c5494f213bd34769a9274e792848fd9ddbb0522 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 2 Jan 2026 00:19:09 +0200 Subject: [PATCH 2/9] Add test to verify that `GITHUB_TOKEN` and `app_auth` are not set at the same time Signed-off-by: Timo Sand --- github/provider_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/github/provider_test.go b/github/provider_test.go index 5387edcb23..5b5eca20a8 100644 --- a/github/provider_test.go +++ b/github/provider_test.go @@ -241,4 +241,29 @@ func TestAccProviderConfigure(t *testing.T) { }, }) }) + t.Run("should not allow app_auth and GITHUB_TOKEN to be configured", func(t *testing.T) { + config := fmt.Sprintf(` + provider "github" { + owner = "%s" + app_auth { + id = "1234567890" + installation_id = "1234567890" + pem_file = "1234567890" + } + } + + data "github_ip_ranges" "test" {} + `, testAccConf.owner) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t); t.Setenv("GITHUB_TOKEN", "1234567890") }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("\"app_auth\": conflicts with token"), + }, + }, + }) + }) } From 0d2f2c205aa4f530f6bdbb12000247b577395e0c Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 2 Jan 2026 00:21:50 +0200 Subject: [PATCH 3/9] Ensure `max_per_page` test actually runs Signed-off-by: Timo Sand --- github/provider_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/github/provider_test.go b/github/provider_test.go index 5b5eca20a8..0a2406529e 100644 --- a/github/provider_test.go +++ b/github/provider_test.go @@ -192,11 +192,15 @@ func TestAccProviderConfigure(t *testing.T) { }) t.Run("can be configured with max per page", func(t *testing.T) { - config := ` + testMaxPerPage := 101 + config := fmt.Sprintf(` provider "github" { owner = "%s" - max_per_page = 100 - }` + max_per_page = %d + } + + data "github_ip_ranges" "test" {} + `, testAccConf.owner, testMaxPerPage) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, @@ -206,8 +210,8 @@ func TestAccProviderConfigure(t *testing.T) { Config: config, ExpectNonEmptyPlan: false, Check: func(_ *terraform.State) error { - if maxPerPage != 100 { - return fmt.Errorf("max_per_page should be set to 100, got %d", maxPerPage) + if maxPerPage != testMaxPerPage { + return fmt.Errorf("max_per_page should be set to %d, got %d", testMaxPerPage, maxPerPage) } return nil }, From 7fac1a6ce661bb8f05e2fa8606acf560950280ed Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 2 Jan 2026 00:25:47 +0200 Subject: [PATCH 4/9] Ensure `max_retries` is actually tested Signed-off-by: Timo Sand --- github/provider_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/github/provider_test.go b/github/provider_test.go index 0a2406529e..ca682c5523 100644 --- a/github/provider_test.go +++ b/github/provider_test.go @@ -173,11 +173,15 @@ func TestAccProviderConfigure(t *testing.T) { }) t.Run("can be configured with max retries", func(t *testing.T) { + testMaxRetries := -1 config := fmt.Sprintf(` provider "github" { owner = "%s" - max_retries = 3 - }`, testAccConf.owner) + max_retries = %d + } + + data "github_ip_ranges" "test" {} + `, testAccConf.owner, testMaxRetries) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, @@ -186,6 +190,7 @@ func TestAccProviderConfigure(t *testing.T) { { Config: config, ExpectNonEmptyPlan: false, + ExpectError: regexp.MustCompile("max_retries must be greater than or equal to 0"), }, }, }) From 1cc898de547b1ee25e0506cb715b5e1822e33fb1 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 2 Jan 2026 00:28:18 +0200 Subject: [PATCH 5/9] Fix indentation Signed-off-by: Timo Sand --- github/provider_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/github/provider_test.go b/github/provider_test.go index ca682c5523..ea3de2a781 100644 --- a/github/provider_test.go +++ b/github/provider_test.go @@ -54,7 +54,7 @@ func TestAccProviderConfigure(t *testing.T) { t.Run("can be configured to run anonymously", func(t *testing.T) { config := ` provider "github" { - token = "" + token = "" } data "github_ip_ranges" "test" {} ` @@ -78,7 +78,7 @@ func TestAccProviderConfigure(t *testing.T) { insecure = true } data "github_ip_ranges" "test" {} - ` + ` resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -180,8 +180,8 @@ func TestAccProviderConfigure(t *testing.T) { max_retries = %d } - data "github_ip_ranges" "test" {} - `, testAccConf.owner, testMaxRetries) + data "github_ip_ranges" "test" {} + `, testAccConf.owner, testMaxRetries) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, From a4d10fd6633cfb515c55d2f49e1a318b099c3654 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 2 Jan 2026 00:41:39 +0200 Subject: [PATCH 6/9] Use `ExactlyOneOf` instead of `ConflictsWith` Signed-off-by: Timo Sand --- github/provider.go | 20 ++++++++++---------- github/provider_test.go | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/github/provider.go b/github/provider.go index a1a542e103..303c3c99b1 100644 --- a/github/provider.go +++ b/github/provider.go @@ -18,11 +18,11 @@ func Provider() *schema.Provider { p := &schema.Provider{ Schema: map[string]*schema.Schema{ "token": { - Type: schema.TypeString, - Optional: true, - DefaultFunc: schema.EnvDefaultFunc("GITHUB_TOKEN", nil), - Description: descriptions["token"], - ConflictsWith: []string{"app_auth"}, + Type: schema.TypeString, + Optional: true, + DefaultFunc: schema.EnvDefaultFunc("GITHUB_TOKEN", nil), + Description: descriptions["token"], + ExactlyOneOf: []string{"app_auth"}, }, "owner": { Type: schema.TypeString, @@ -94,11 +94,11 @@ func Provider() *schema.Provider { Description: descriptions["parallel_requests"], }, "app_auth": { - Type: schema.TypeList, - Optional: true, - MaxItems: 1, - Description: descriptions["app_auth"], - ConflictsWith: []string{"token"}, + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Description: descriptions["app_auth"], + ExactlyOneOf: []string{"token"}, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "id": { diff --git a/github/provider_test.go b/github/provider_test.go index ea3de2a781..6382c42c35 100644 --- a/github/provider_test.go +++ b/github/provider_test.go @@ -245,7 +245,7 @@ func TestAccProviderConfigure(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile("\"app_auth\": conflicts with token"), + ExpectError: regexp.MustCompile("only one of `app_auth,token` can be specified"), }, }, }) @@ -270,7 +270,7 @@ func TestAccProviderConfigure(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile("\"app_auth\": conflicts with token"), + ExpectError: regexp.MustCompile("only one of `app_auth,token` can be specified"), }, }, }) From d5d8d6e0fc6df9ff5745c9ed44d4f1fad77abc50 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 2 Jan 2026 08:52:28 +0200 Subject: [PATCH 7/9] Add some more infor logging Signed-off-by: Timo Sand --- github/provider.go | 1 + 1 file changed, 1 insertion(+) diff --git a/github/provider.go b/github/provider.go index 303c3c99b1..3a3b24863c 100644 --- a/github/provider.go +++ b/github/provider.go @@ -416,6 +416,7 @@ func providerConfigure(p *schema.Provider) schema.ConfigureContextFunc { } if token == "" { + log.Printf("[INFO] No token found, using GitHub CLI to get token from hostname %s", baseURL.Host) token = tokenFromGHCLI(baseURL) } From b45ac729a9928871c6772c575ec9ec11b3e03d67 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 2 Jan 2026 08:53:21 +0200 Subject: [PATCH 8/9] Fix brackets Signed-off-by: Timo Sand --- github/provider_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/github/provider_test.go b/github/provider_test.go index 6382c42c35..e0cbfde27e 100644 --- a/github/provider_test.go +++ b/github/provider_test.go @@ -131,25 +131,25 @@ func TestAccProviderConfigure(t *testing.T) { }, }, }) + }) - t.Run("can be configured with an organization account legacy", func(t *testing.T) { - config := fmt.Sprintf(` + t.Run("can be configured with an organization account legacy", func(t *testing.T) { + config := fmt.Sprintf(` provider "github" { token = "%s" organization = "%s" }`, testAccConf.token, testAccConf.owner) - resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessHasOrgs(t) }, - ProviderFactories: providerFactories, - Steps: []resource.TestStep{ - { - Config: config, - PlanOnly: true, - ExpectNonEmptyPlan: false, - }, + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + PlanOnly: true, + ExpectNonEmptyPlan: false, }, - }) + }, }) }) From 0ecb31aad722f73e126ea484bd2ff3c0dbc73353 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 2 Jan 2026 08:54:30 +0200 Subject: [PATCH 9/9] Update Org test to ensure it actually tests Signed-off-by: Timo Sand --- github/provider_test.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/github/provider_test.go b/github/provider_test.go index e0cbfde27e..6d05f3fa15 100644 --- a/github/provider_test.go +++ b/github/provider_test.go @@ -117,17 +117,20 @@ func TestAccProviderConfigure(t *testing.T) { config := fmt.Sprintf(` provider "github" { token = "%s" - owner = "%s" - }`, testAccConf.token, testAccConf.owner) + owner = "%[2]s" + } + + data "github_organization" "test" { + name = "%[2]s" + } + `, testAccConf.token, testAccConf.owner) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: config, - PlanOnly: true, - ExpectNonEmptyPlan: false, + Config: config, }, }, })