From ddda76f3967a32ecf48ba3bda7114d8873a5e2e2 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 27 Mar 2026 22:37:34 +0200 Subject: [PATCH 01/15] Refactor tests to use testing plugin Signed-off-by: Timo Sand --- github/resource_github_repository_test.go | 588 ++++++++-------------- 1 file changed, 223 insertions(+), 365 deletions(-) diff --git a/github/resource_github_repository_test.go b/github/resource_github_repository_test.go index cc6df3c64d..4864019760 100644 --- a/github/resource_github_repository_test.go +++ b/github/resource_github_repository_test.go @@ -2,15 +2,18 @@ package github import ( "fmt" + "regexp" "strings" "testing" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" + "github.com/hashicorp/terraform-plugin-testing/statecheck" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" ) func TestAccGithubRepository(t *testing.T) { @@ -38,36 +41,19 @@ func TestAccGithubRepository(t *testing.T) { } `, testRepoName, testAccConf.testRepositoryVisibility) - check := resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.test", "has_issues", - "true", - ), - resource.TestCheckResourceAttr( - "github_repository.test", "has_discussions", - "true", - ), - resource.TestCheckResourceAttr( - "github_repository.test", "allow_auto_merge", - "true", - ), - resource.TestCheckResourceAttr( - "github_repository.test", "merge_commit_title", - "MERGE_MESSAGE", - ), - resource.TestCheckResourceAttr( - "github_repository.test", "web_commit_signoff_required", - "true", - ), - ) - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: check, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("has_issues"), knownvalue.Bool(true)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("has_discussions"), knownvalue.Bool(true)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("allow_auto_merge"), knownvalue.Bool(true)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("merge_commit_title"), knownvalue.StringExact("MERGE_MESSAGE")), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("web_commit_signoff_required"), knownvalue.Bool(true)), + }, }, }, }) @@ -86,36 +72,16 @@ func TestAccGithubRepository(t *testing.T) { } `, oldName, randomID, testAccConf.testRepositoryVisibility) - checks := map[string]resource.TestCheckFunc{ - "before": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.test", "name", - oldName, - ), - resource.ComposeTestCheckFunc( - testCheckResourceAttrContains("github_repository.test", "full_name", - oldName), - ), - ), - "after": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.test", "name", - newName, - ), - resource.ComposeTestCheckFunc( - testCheckResourceAttrContains("github_repository.test", "full_name", - newName), - ), - ), - } - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: checks["before"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("name"), knownvalue.StringExact(oldName)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("full_name"), knownvalue.StringRegexp(regexp.MustCompile(regexp.QuoteMeta(oldName)))), + }, }, { // Rename the repo to something else @@ -123,7 +89,10 @@ func TestAccGithubRepository(t *testing.T) { config, oldName, newName, 1), - Check: checks["after"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("name"), knownvalue.StringExact(newName)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("full_name"), knownvalue.StringRegexp(regexp.MustCompile(regexp.QuoteMeta(newName)))), + }, }, }, }) @@ -141,17 +110,15 @@ func TestAccGithubRepository(t *testing.T) { } `, testRepoName, testAccConf.testRepositoryVisibility) - check := resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrSet("github_repository.test", "name"), - ) - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: check, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("name"), knownvalue.NotNull()), + }, }, { ResourceName: "github_repository.test", @@ -181,15 +148,15 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, "false", testAccConf.testRepositoryVisibility), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "archived", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("archived"), knownvalue.Bool(false)), + }, }, { Config: fmt.Sprintf(config, testRepoName, "true", testAccConf.testRepositoryVisibility), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "archived", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("archived"), knownvalue.Bool(true)), + }, }, }, }) @@ -207,34 +174,23 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility) - checks := map[string]resource.TestCheckFunc{ - "before": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.test", "has_projects", - "false", - ), - ), - "after": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.test", "has_projects", - "true", - ), - ), - } - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: checks["before"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("has_projects"), knownvalue.Bool(false)), + }, }, { Config: strings.Replace(config, `has_projects = false`, `has_projects = true`, 1), - Check: checks["after"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("has_projects"), knownvalue.Bool(true)), + }, }, }, }) @@ -258,40 +214,31 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility) - checks := map[string]resource.TestCheckFunc{ - "before": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.test", "default_branch", - "main", - ), - ), - "after": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.test", "default_branch", - "default", - ), - ), - } - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: checks["before"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("default_branch"), knownvalue.StringExact("main")), + }, }, // Test changing default_branch { Config: strings.Replace(config, `default_branch = "main"`, `default_branch = "default"`, 1), - Check: checks["after"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("default_branch"), knownvalue.StringExact("default")), + }, }, // Test changing default_branch back to main again { Config: config, - Check: checks["before"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("default_branch"), knownvalue.StringExact("main")), + }, }, }, }) @@ -312,12 +259,9 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility) - check := resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.test", "default_branch", - "main", - ), - ) + defaultBranchChecks := []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("default_branch"), knownvalue.StringExact("main")), + } resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, @@ -325,8 +269,8 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ // Test creation with default_branch set { - Config: config, - Check: check, + Config: config, + ConfigStateChecks: defaultBranchChecks, }, // Test that changing another property does not try to set // default_branch (which would crash). @@ -334,7 +278,7 @@ resource "github_repository" "test" { Config: strings.Replace(config, `acceptance tests`, `acceptance test`, 1), - Check: check, + ConfigStateChecks: defaultBranchChecks, }, }, }) @@ -353,24 +297,16 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility) - check := resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.test", "license_template", - "ms-pl", - ), - resource.TestCheckResourceAttr( - "github_repository.test", "gitignore_template", - "C++", - ), - ) - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: check, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("license_template"), knownvalue.StringExact("ms-pl")), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("gitignore_template"), knownvalue.StringExact("C++")), + }, }, }, }) @@ -396,15 +332,15 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, topicsBefore, testAccConf.testRepositoryVisibility), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "topics.#", "2"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("topics"), knownvalue.SetSizeExact(2)), + }, }, { Config: fmt.Sprintf(config, testRepoName, topicsAfter, testAccConf.testRepositoryVisibility), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "topics.#", "3"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("topics"), knownvalue.SetSizeExact(3)), + }, }, }, }) @@ -432,9 +368,9 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "is_template", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("is_template"), knownvalue.Bool(false)), + }, }, }, }) @@ -462,9 +398,9 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "is_template", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("is_template"), knownvalue.Bool(false)), + }, }, }, }) @@ -490,15 +426,15 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, "false", testAccConf.testRepositoryVisibility), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "archived", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("archived"), knownvalue.Bool(false)), + }, }, { Config: fmt.Sprintf(config, testRepoName, "true", testAccConf.testRepositoryVisibility), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "archived", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("archived"), knownvalue.Bool(true)), + }, }, }, }) @@ -523,10 +459,10 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "visibility", "private"), - resource.TestCheckResourceAttr("github_repository.test", "allow_forking", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("visibility"), knownvalue.StringExact("private")), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("allow_forking"), knownvalue.Bool(true)), + }, }, }, }) @@ -551,10 +487,10 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "visibility", "private"), - resource.TestCheckResourceAttr("github_repository.test", "allow_forking", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("visibility"), knownvalue.StringExact("private")), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("allow_forking"), knownvalue.Bool(false)), + }, }, }, }) @@ -577,10 +513,10 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "visibility", "private"), - resource.TestCheckResourceAttrSet("github_repository.test", "allow_forking"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("visibility"), knownvalue.StringExact("private")), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("allow_forking"), knownvalue.NotNull()), + }, }, }, }) @@ -621,24 +557,24 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "visibility", "public"), - resource.TestCheckResourceAttr("github_repository.test", "allow_forking", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("visibility"), knownvalue.StringExact("public")), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("allow_forking"), knownvalue.Bool(true)), + }, }, { Config: configPrivate, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "visibility", "private"), - resource.TestCheckResourceAttr("github_repository.test", "allow_forking", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("visibility"), knownvalue.StringExact("private")), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("allow_forking"), knownvalue.Bool(false)), + }, }, { Config: configPrivateForking, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "visibility", "private"), - resource.TestCheckResourceAttr("github_repository.test", "allow_forking", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("visibility"), knownvalue.StringExact("private")), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("allow_forking"), knownvalue.Bool(true)), + }, }, }, }) @@ -663,9 +599,9 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "vulnerability_alerts", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("vulnerability_alerts"), knownvalue.Bool(true)), + }, }, }, }) @@ -690,9 +626,9 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "vulnerability_alerts", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("vulnerability_alerts"), knownvalue.Bool(false)), + }, }, }, }) @@ -709,15 +645,14 @@ resource "github_repository" "test" { } `, repoName, testAccConf.testRepositoryVisibility) + // NOTE: terraform-plugin-testing does not support asserting the nonexistence of a value + // (no equivalent to the legacy TestCheckNoResourceAttr). We only verify creation succeeds. resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckNoResourceAttr("github_repository.test", "vulnerability_alerts"), // This will error if it's run as an unprivileged user but that shouldn't ever happen - ), }, }, }) @@ -742,15 +677,15 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, repoName, testAccConf.testRepositoryVisibility, false), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "vulnerability_alerts", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("vulnerability_alerts"), knownvalue.Bool(false)), + }, }, { Config: fmt.Sprintf(config, repoName, testAccConf.testRepositoryVisibility, true), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "vulnerability_alerts", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("vulnerability_alerts"), knownvalue.Bool(true)), + }, }, }, }) @@ -780,17 +715,17 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, mergeCommitTitle, mergeCommitMessage, testAccConf.testRepositoryVisibility), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "merge_commit_title", mergeCommitTitle), - resource.TestCheckResourceAttr("github_repository.test", "merge_commit_message", mergeCommitMessage), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("merge_commit_title"), knownvalue.StringExact(mergeCommitTitle)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("merge_commit_message"), knownvalue.StringExact(mergeCommitMessage)), + }, }, { Config: fmt.Sprintf(config, testRepoName, updatedMergeCommitTitle, updatedMergeCommitMessage, testAccConf.testRepositoryVisibility), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "merge_commit_title", updatedMergeCommitTitle), - resource.TestCheckResourceAttr("github_repository.test", "merge_commit_message", updatedMergeCommitMessage), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("merge_commit_title"), knownvalue.StringExact(updatedMergeCommitTitle)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("merge_commit_message"), knownvalue.StringExact(updatedMergeCommitMessage)), + }, }, }, }) @@ -815,28 +750,23 @@ resource "github_repository" "test" { } ` - checks := map[string]resource.TestCheckFunc{ - "before": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "squash_merge_commit_title", squashMergeCommitTitle), - resource.TestCheckResourceAttr("github_repository.test", "squash_merge_commit_message", squashMergeCommitMessage), - ), - "after": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "squash_merge_commit_title", updatedSquashMergeCommitTitle), - resource.TestCheckResourceAttr("github_repository.test", "squash_merge_commit_message", updatedSquashMergeCommitMessage), - ), - } - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, squashMergeCommitTitle, squashMergeCommitMessage, testAccConf.testRepositoryVisibility), - Check: checks["before"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("squash_merge_commit_title"), knownvalue.StringExact(squashMergeCommitTitle)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("squash_merge_commit_message"), knownvalue.StringExact(squashMergeCommitMessage)), + }, }, { Config: fmt.Sprintf(config, testRepoNameAfter, updatedSquashMergeCommitTitle, updatedSquashMergeCommitMessage, testAccConf.testRepositoryVisibility), - Check: checks["after"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("squash_merge_commit_title"), knownvalue.StringExact(updatedSquashMergeCommitTitle)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("squash_merge_commit_message"), knownvalue.StringExact(updatedSquashMergeCommitMessage)), + }, }, }, }) @@ -902,10 +832,14 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "pages.0.source.0.branch", "main"), - resource.TestCheckResourceAttr("github_repository.test", "pages.0.source.0.path", "/"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", + tfjsonpath.New("pages").AtSliceIndex(0).AtMapKey("source").AtSliceIndex(0).AtMapKey("branch"), + knownvalue.StringExact("main")), + statecheck.ExpectKnownValue("github_repository.test", + tfjsonpath.New("pages").AtSliceIndex(0).AtMapKey("source").AtSliceIndex(0).AtMapKey("path"), + knownvalue.StringExact("/")), + }, }, }, }) @@ -931,9 +865,13 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckNoResourceAttr("github_repository.test", "pages.0.source.#"), - ), + // NOTE: terraform-plugin-testing does not support asserting the nonexistence of a value; + // TypeList nil is represented as empty slice in JSON state + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", + tfjsonpath.New("pages").AtSliceIndex(0).AtMapKey("source"), + knownvalue.ListSizeExact(0)), + }, }, }, }) @@ -974,20 +912,21 @@ resource "github_repository" "test" { } `, testRepoName) + securityPath := tfjsonpath.New("security_and_analysis").AtSliceIndex(0) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "security_and_analysis.0.advanced_security.0.status", "enabled"), - resource.TestCheckResourceAttr("github_repository.test", "security_and_analysis.0.code_security.0.status", "enabled"), - resource.TestCheckResourceAttr("github_repository.test", "security_and_analysis.0.secret_scanning.0.status", "enabled"), - resource.TestCheckResourceAttr("github_repository.test", "security_and_analysis.0.secret_scanning_push_protection.0.status", "enabled"), - resource.TestCheckResourceAttr("github_repository.test", "security_and_analysis.0.secret_scanning_ai_detection.0.status", "enabled"), - resource.TestCheckResourceAttr("github_repository.test", "security_and_analysis.0.secret_scanning_non_provider_patterns.0.status", "enabled"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", securityPath.AtMapKey("advanced_security").AtSliceIndex(0).AtMapKey("status"), knownvalue.StringExact("enabled")), + statecheck.ExpectKnownValue("github_repository.test", securityPath.AtMapKey("code_security").AtSliceIndex(0).AtMapKey("status"), knownvalue.StringExact("enabled")), + statecheck.ExpectKnownValue("github_repository.test", securityPath.AtMapKey("secret_scanning").AtSliceIndex(0).AtMapKey("status"), knownvalue.StringExact("enabled")), + statecheck.ExpectKnownValue("github_repository.test", securityPath.AtMapKey("secret_scanning_push_protection").AtSliceIndex(0).AtMapKey("status"), knownvalue.StringExact("enabled")), + statecheck.ExpectKnownValue("github_repository.test", securityPath.AtMapKey("secret_scanning_ai_detection").AtSliceIndex(0).AtMapKey("status"), knownvalue.StringExact("enabled")), + statecheck.ExpectKnownValue("github_repository.test", securityPath.AtMapKey("secret_scanning_non_provider_patterns").AtSliceIndex(0).AtMapKey("status"), knownvalue.StringExact("enabled")), + }, }, }, }) @@ -1013,6 +952,7 @@ resource "github_repository" "test" { } `, testRepoName) + securityPath := tfjsonpath.New("security_and_analysis").AtSliceIndex(0) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) @@ -1022,10 +962,10 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "security_and_analysis.0.secret_scanning.0.status", "enabled"), - resource.TestCheckResourceAttr("github_repository.test", "security_and_analysis.0.secret_scanning_push_protection.0.status", "disabled"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", securityPath.AtMapKey("secret_scanning").AtSliceIndex(0).AtMapKey("status"), knownvalue.StringExact("enabled")), + statecheck.ExpectKnownValue("github_repository.test", securityPath.AtMapKey("secret_scanning_push_protection").AtSliceIndex(0).AtMapKey("status"), knownvalue.StringExact("disabled")), + }, }, }, }) @@ -1047,9 +987,9 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.private", "visibility", "private"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.private", tfjsonpath.New("visibility"), knownvalue.StringExact("private")), + }, }, }, }) @@ -1071,9 +1011,9 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "visibility", "internal"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("visibility"), knownvalue.StringExact("internal")), + }, }, }, }) @@ -1096,15 +1036,15 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, "public"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.public", "visibility", "public"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.public", tfjsonpath.New("visibility"), knownvalue.StringExact("public")), + }, }, { Config: fmt.Sprintf(config, testRepoName, "private"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.public", "visibility", "private"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.public", tfjsonpath.New("visibility"), knownvalue.StringExact("private")), + }, }, }, }) @@ -1120,37 +1060,24 @@ resource "github_repository" "test" { } `, testRepoName) - checks := map[string]resource.TestCheckFunc{ - "before": resource.ComposeTestCheckFunc( - resource.TestCheckNoResourceAttr( - "github_repository.test", "vulnerability_alerts", - ), - ), - "after": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.test", "vulnerability_alerts", - "true", - ), - resource.TestCheckResourceAttr( - "github_repository.test", "visibility", - "private", - ), - ), - } - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: checks["before"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("vulnerability_alerts"), knownvalue.Null()), + }, }, { Config: strings.Replace(config, `}`, "vulnerability_alerts = true\n}", 1), - Check: checks["after"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("vulnerability_alerts"), knownvalue.Bool(true)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("visibility"), knownvalue.StringExact("private")), + }, }, }, }) @@ -1166,29 +1093,24 @@ resource "github_repository" "test" { } `, testRepoName) - checks := map[string]resource.TestCheckFunc{ - "before": resource.ComposeTestCheckFunc( - resource.TestCheckNoResourceAttr("github_repository.test", "vulnerability_alerts"), - ), - "after": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "vulnerability_alerts", "true"), - resource.TestCheckResourceAttr("github_repository.test", "visibility", "private"), - ), - } - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessMode(t, enterprise) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: checks["before"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("vulnerability_alerts"), knownvalue.Null()), + }, }, { Config: strings.Replace(config, `}`, "vulnerability_alerts = true\n}", 1), - Check: checks["after"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("vulnerability_alerts"), knownvalue.Bool(true)), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("visibility"), knownvalue.StringExact("private")), + }, }, }, }) @@ -1208,24 +1130,16 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testPublicTemplateRepositoryOwner, testAccConf.testPublicTemplateRepository) - check := resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.private", "visibility", - "private", - ), - resource.TestCheckResourceAttr( - "github_repository.private", "private", - "true", - ), - ) - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t); skipIfEMUEnterprise(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: check, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.private", tfjsonpath.New("visibility"), knownvalue.StringExact("private")), + statecheck.ExpectKnownValue("github_repository.private", tfjsonpath.New("private"), knownvalue.Bool(true)), + }, }, }, }) @@ -1251,10 +1165,10 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "visibility", "internal"), - resource.TestCheckResourceAttr("github_repository.test", "private", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("visibility"), knownvalue.StringExact("internal")), + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("private"), knownvalue.Bool(true)), + }, }, }, }) @@ -1277,9 +1191,9 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, "true"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "web_commit_signoff_required", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("web_commit_signoff_required"), knownvalue.Bool(true)), + }, }, }, }) @@ -1302,9 +1216,9 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, "false"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "web_commit_signoff_required", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("web_commit_signoff_required"), knownvalue.Bool(false)), + }, }, }, }) @@ -1326,9 +1240,9 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "web_commit_signoff_required", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("web_commit_signoff_required"), knownvalue.Bool(false)), + }, }, }, }) @@ -1354,9 +1268,9 @@ resource "github_repository" "test" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, "foo"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.test", "web_commit_signoff_required", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("web_commit_signoff_required"), knownvalue.Bool(true)), + }, }, { Config: fmt.Sprintf(config, testRepoName, "bar"), @@ -1385,9 +1299,9 @@ resource "github_repository" "private" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, "foo"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.private", "allow_forking", "false"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.private", tfjsonpath.New("allow_forking"), knownvalue.Bool(false)), + }, }, { Config: fmt.Sprintf(config, testRepoName, "bar"), @@ -1416,9 +1330,9 @@ resource "github_repository" "private" { Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, testRepoName, "foo"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_repository.private", "vulnerability_alerts", "true"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.private", tfjsonpath.New("vulnerability_alerts"), knownvalue.Bool(true)), + }, }, { Config: fmt.Sprintf(config, testRepoName, "bar"), @@ -1484,32 +1398,19 @@ func Test_expandPages(t *testing.T) { } `, testRepoName) - check := resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.forked", "fork", - "true", - ), - resource.TestCheckResourceAttrSet( - "github_repository.forked", "html_url", - ), - resource.TestCheckResourceAttrSet( - "github_repository.forked", "ssh_clone_url", - ), - resource.TestCheckResourceAttrSet( - "github_repository.forked", "git_clone_url", - ), - resource.TestCheckResourceAttrSet( - "github_repository.forked", "http_clone_url", - ), - ) - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, - Check: check, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.forked", tfjsonpath.New("fork"), knownvalue.StringExact("true")), + statecheck.ExpectKnownValue("github_repository.forked", tfjsonpath.New("html_url"), knownvalue.NotNull()), + statecheck.ExpectKnownValue("github_repository.forked", tfjsonpath.New("ssh_clone_url"), knownvalue.NotNull()), + statecheck.ExpectKnownValue("github_repository.forked", tfjsonpath.New("git_clone_url"), knownvalue.NotNull()), + statecheck.ExpectKnownValue("github_repository.forked", tfjsonpath.New("http_clone_url"), knownvalue.NotNull()), + }, }, }, }) @@ -1542,48 +1443,25 @@ func Test_expandPages(t *testing.T) { } `, testRepoName) - checks := map[string]resource.TestCheckFunc{ - "before": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.forked_update", "description", - "Initial description for forked repo", - ), - resource.TestCheckResourceAttr( - "github_repository.forked_update", "has_wiki", - "true", - ), - resource.TestCheckResourceAttr( - "github_repository.forked_update", "has_issues", - "false", - ), - ), - "after": resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr( - "github_repository.forked_update", "description", - "Updated description for forked repo", - ), - resource.TestCheckResourceAttr( - "github_repository.forked_update", "has_wiki", - "false", - ), - resource.TestCheckResourceAttr( - "github_repository.forked_update", "has_issues", - "true", - ), - ), - } - resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: initialConfig, - Check: checks["before"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.forked_update", tfjsonpath.New("description"), knownvalue.StringExact("Initial description for forked repo")), + statecheck.ExpectKnownValue("github_repository.forked_update", tfjsonpath.New("has_wiki"), knownvalue.Bool(true)), + statecheck.ExpectKnownValue("github_repository.forked_update", tfjsonpath.New("has_issues"), knownvalue.Bool(false)), + }, }, { Config: updatedConfig, - Check: checks["after"], + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.forked_update", tfjsonpath.New("description"), knownvalue.StringExact("Updated description for forked repo")), + statecheck.ExpectKnownValue("github_repository.forked_update", tfjsonpath.New("has_wiki"), knownvalue.Bool(false)), + statecheck.ExpectKnownValue("github_repository.forked_update", tfjsonpath.New("has_issues"), knownvalue.Bool(true)), + }, }, { ResourceName: "github_repository.forked_update", @@ -1659,26 +1537,6 @@ func TestResourceGithubParseFullName(t *testing.T) { }) } -func testCheckResourceAttrContains(resourceName, attributeName, substring string) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[resourceName] - if !ok { - return fmt.Errorf("Resource not found: %s", resourceName) - } - - value, ok := rs.Primary.Attributes[attributeName] - if !ok { - return fmt.Errorf("Attribute not found: %s", attributeName) - } - - if !strings.Contains(value, substring) { - return fmt.Errorf("Attribute '%s' does not contain '%s'", value, substring) - } - - return nil - } -} - func TestGithubRepositoryNameFailsValidationWhenOverMaxCharacters(t *testing.T) { resource := resourceGithubRepository() schema := resource.Schema["name"] From 95f6856f6aefdd43b87fc1554bfe81f3c8d7f4a2 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 28 Mar 2026 12:02:36 +0200 Subject: [PATCH 02/15] Refactor to use `tflog` Signed-off-by: Timo Sand --- github/resource_github_repository.go | 29 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index f6408bffe6..003b80fda0 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -4,12 +4,12 @@ import ( "context" "errors" "fmt" - "log" "net/http" "regexp" "strings" "github.com/google/go-github/v84/github" + "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -700,7 +700,7 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, sourceOwner := d.Get("source_owner").(string) sourceRepo := d.Get("source_repo").(string) requestedName := d.Get("name").(string) - log.Printf("[INFO] Creating fork of %s/%s in %s", sourceOwner, sourceRepo, owner) + tflog.Info(ctx, "Creating fork", map[string]any{"source_owner": sourceOwner, "source_repo": sourceRepo, "owner": owner}) if sourceOwner == "" || sourceRepo == "" { return diag.Errorf("source_owner and source_repo must be provided when forking a repository") @@ -721,26 +721,26 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, // Handle accepted error (202) which means the fork is being created asynchronously acceptedError := &github.AcceptedError{} if errors.As(err, &acceptedError) { - log.Printf("[INFO] Fork is being created asynchronously") + tflog.Info(ctx, "Fork is being created asynchronously") // Despite the 202 status, the API should still return preliminary fork information if fork == nil { return diag.Errorf("fork information not available after accepted status") } - log.Printf("[DEBUG] Fork name: %s", fork.GetName()) + tflog.Debug(ctx, "Fork name", map[string]any{"name": fork.GetName()}) } else { return diag.Errorf("failed to create fork: %s", err.Error()) } } else if resp != nil { - log.Printf("[DEBUG] Fork response status: %d", resp.StatusCode) + tflog.Debug(ctx, "Fork response status", map[string]any{"status": resp.StatusCode}) } if fork == nil { return diag.Errorf("fork creation failed - no repository returned") } - log.Printf("[INFO] Fork created with name: %s", fork.GetName()) + tflog.Info(ctx, "Fork created", map[string]any{"name": fork.GetName()}) d.SetId(fork.GetName()) - log.Printf("[DEBUG] Set resource ID to just the name: %s", d.Id()) + tflog.Debug(ctx, "Set resource ID to just the name", map[string]any{"resource_id": d.Id()}) _ = d.Set("name", fork.GetName()) _ = d.Set("full_name", fork.GetFullName()) // Add the full name for reference @@ -808,8 +808,7 @@ func resourceGithubRepositoryRead(ctx context.Context, d *schema.ResourceData, m return nil } if ghErr.Response.StatusCode == http.StatusNotFound { - log.Printf("[INFO] Removing repository %s/%s from state because it no longer exists in GitHub", - owner, repoName) + tflog.Info(ctx, "Removing repository from state because it no longer exists in GitHub", map[string]any{"owner": owner, "name": repoName}) d.SetId("") return nil } @@ -920,7 +919,7 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, // Can only update a repository if it is not archived or the update is to // archive the repository (unarchiving is not supported by the GitHub API) if d.Get("archived").(bool) && !d.HasChange("archived") { - log.Printf("[INFO] Skipping update of archived repository") + tflog.Info(ctx, "Skipping update of archived repository") return nil } @@ -940,7 +939,7 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, if !d.HasChange("security_and_analysis") { repoReq.SecurityAndAnalysis = nil - log.Print("[DEBUG] No security_and_analysis update required. Removing this field from the payload.") + tflog.Debug(ctx, "No security_and_analysis update required, removing from payload") } // The documentation for `default_branch` states: "This can only be set @@ -1009,7 +1008,7 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, repoReq.Visibility = new(visibility) repoReq.AllowForking = allowForking - log.Printf("[DEBUG] Updating repository visibility from %s to %s", repo.GetVisibility(), visibility) + tflog.Debug(ctx, "Updating repository visibility", map[string]any{"from": repo.GetVisibility(), "to": visibility}) _, resp, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) if err != nil { if resp.StatusCode != 422 || !strings.Contains(err.Error(), fmt.Sprintf("Visibility is already %s", visibility)) { @@ -1030,20 +1029,20 @@ func resourceGithubRepositoryDelete(ctx context.Context, d *schema.ResourceData, archiveOnDestroy := d.Get("archive_on_destroy").(bool) if archiveOnDestroy { if d.Get("archived").(bool) { - log.Printf("[DEBUG] Repository already archived, nothing to do on delete: %s/%s", owner, repoName) + tflog.Debug(ctx, "Repository already archived, nothing to do on delete", map[string]any{"owner": owner, "name": repoName}) return nil } else { if err := d.Set("archived", true); err != nil { return diag.FromErr(err) } repoReq := resourceGithubRepositoryObject(d) - log.Printf("[DEBUG] Archiving repository on delete: %s/%s", owner, repoName) + tflog.Debug(ctx, "Archiving repository on delete", map[string]any{"owner": owner, "name": repoName}) _, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) return diag.FromErr(err) } } - log.Printf("[DEBUG] Deleting repository: %s/%s", owner, repoName) + tflog.Debug(ctx, "Deleting repository", map[string]any{"owner": owner, "name": repoName}) _, err := client.Repositories.Delete(ctx, owner, repoName) return diag.FromErr(err) } From 24a737348dcc0eaa7727e99075304eccbbd47a2c Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 28 Mar 2026 17:02:18 +0200 Subject: [PATCH 03/15] Remove gate for `archived` repos in Read. All these fields are being returned by the API even for archived repos Signed-off-by: Timo Sand --- github/resource_github_repository.go | 32 ++++++++++++++-------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index 003b80fda0..b0fb55bb04 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -840,22 +840,22 @@ func resourceGithubRepositoryRead(ctx context.Context, d *schema.ResourceData, m _ = d.Set("node_id", repo.GetNodeID()) _ = d.Set("repo_id", repo.GetID()) - // TODO: Validate this behavior as I can see these fields being returned even when archived - // GitHub API doesn't respond following parameters when repository is archived - if !d.Get("archived").(bool) { - _ = d.Set("allow_auto_merge", repo.GetAllowAutoMerge()) - _ = d.Set("allow_merge_commit", repo.GetAllowMergeCommit()) - _ = d.Set("allow_rebase_merge", repo.GetAllowRebaseMerge()) - _ = d.Set("allow_squash_merge", repo.GetAllowSquashMerge()) - _ = d.Set("allow_update_branch", repo.GetAllowUpdateBranch()) - _ = d.Set("allow_forking", repo.GetAllowForking()) - _ = d.Set("delete_branch_on_merge", repo.GetDeleteBranchOnMerge()) - _ = d.Set("web_commit_signoff_required", repo.GetWebCommitSignoffRequired()) - _ = d.Set("has_downloads", repo.GetHasDownloads()) - _ = d.Set("merge_commit_message", repo.GetMergeCommitMessage()) - _ = d.Set("merge_commit_title", repo.GetMergeCommitTitle()) - _ = d.Set("squash_merge_commit_message", repo.GetSquashMergeCommitMessage()) - _ = d.Set("squash_merge_commit_title", repo.GetSquashMergeCommitTitle()) + if err := errors.Join( + d.Set("allow_auto_merge", repo.GetAllowAutoMerge()), + d.Set("allow_merge_commit", repo.GetAllowMergeCommit()), + d.Set("allow_rebase_merge", repo.GetAllowRebaseMerge()), + d.Set("allow_squash_merge", repo.GetAllowSquashMerge()), + d.Set("allow_update_branch", repo.GetAllowUpdateBranch()), + d.Set("allow_forking", repo.GetAllowForking()), + d.Set("delete_branch_on_merge", repo.GetDeleteBranchOnMerge()), + d.Set("web_commit_signoff_required", repo.GetWebCommitSignoffRequired()), + d.Set("has_downloads", repo.GetHasDownloads()), + d.Set("merge_commit_message", repo.GetMergeCommitMessage()), + d.Set("merge_commit_title", repo.GetMergeCommitTitle()), + d.Set("squash_merge_commit_message", repo.GetSquashMergeCommitMessage()), + d.Set("squash_merge_commit_title", repo.GetSquashMergeCommitTitle()), + ); err != nil { + tflog.Warn(ctx, "Error setting repository fields, which were previously ignored for archived repositories", map[string]any{"error": err.Error()}) } if repo.GetHasPages() { From c9fbaa9c9b2f80528f2ead5d7dd3e2182bd51a4e Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 28 Mar 2026 17:40:03 +0200 Subject: [PATCH 04/15] Refactor to no longer call CRUD methods directly Signed-off-by: Timo Sand --- github/resource_github_repository.go | 281 +++++++++++++++++---------- 1 file changed, 175 insertions(+), 106 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index b0fb55bb04..b97af9e34d 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -740,14 +740,6 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, tflog.Info(ctx, "Fork created", map[string]any{"name": fork.GetName()}) d.SetId(fork.GetName()) - tflog.Debug(ctx, "Set resource ID to just the name", map[string]any{"resource_id": d.Id()}) - - _ = d.Set("name", fork.GetName()) - _ = d.Set("full_name", fork.GetFullName()) // Add the full name for reference - _ = d.Set("html_url", fork.GetHTMLURL()) - _ = d.Set("ssh_clone_url", fork.GetSSHURL()) - _ = d.Set("git_clone_url", fork.GetGitURL()) - _ = d.Set("http_clone_url", fork.GetCloneURL()) } else { // Create without a repository template var repo *github.Repository @@ -780,7 +772,56 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, } } - return resourceGithubRepositoryUpdate(ctx, d, meta) + // handle visibility updates separately from other fields + visibility := repoReq.GetVisibility() + repoReq.Visibility = nil + + // This change needs to be made with the correct visibility + allowForking := repoReq.AllowForking + if d.HasChanges("visibility", "private") { + repoReq.AllowForking = nil + } + + if !d.HasChange("security_and_analysis") { + repoReq.SecurityAndAnalysis = nil + } + + _, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) + if err != nil { + return diag.FromErr(err) + } + + if v, ok := d.GetOkExists("vulnerability_alerts"); ok { //nolint:staticcheck // SA1019 // We sometimes need to use GetOkExists for booleans + if val, ok := v.(bool); ok { + if err := updateVulnerabilityAlerts(ctx, client, owner, repoName, val); err != nil { + return diag.FromErr(err) + } + } + } + + if d.HasChanges("visibility", "private") { + repoReq.Visibility = new(visibility) + repoReq.AllowForking = allowForking + + _, resp, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) + if err != nil { + if resp.StatusCode != 422 || !strings.Contains(err.Error(), fmt.Sprintf("Visibility is already %s", visibility)) { + return diag.FromErr(err) + } + } + } + + // Fetch final repo state after all mutations and set Computed fields + finalRepo, _, err := client.Repositories.Get(ctx, owner, d.Id()) + if err != nil { + return diag.FromErr(err) + } + + if diags := setRepositoryFieldsFromRepo(ctx, d, client, owner, finalRepo); diags.HasError() { + return diags + } + + return nil } func resourceGithubRepositoryRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { @@ -795,7 +836,6 @@ func resourceGithubRepositoryRead(ctx context.Context, d *schema.ResourceData, m owner = explicitOwner } - ctx = context.WithValue(ctx, ctxId, d.Id()) if !d.IsNewResource() { ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string)) } @@ -816,100 +856,12 @@ func resourceGithubRepositoryRead(ctx context.Context, d *schema.ResourceData, m return diag.FromErr(err) } - _ = d.Set("etag", resp.Header.Get("ETag")) - _ = d.Set("name", repoName) - _ = d.Set("description", repo.GetDescription()) - _ = d.Set("primary_language", repo.GetLanguage()) - _ = d.Set("homepage_url", repo.GetHomepage()) - _ = d.Set("private", repo.GetPrivate()) - _ = d.Set("visibility", repo.GetVisibility()) - _ = d.Set("has_issues", repo.GetHasIssues()) - _ = d.Set("has_discussions", repo.GetHasDiscussions()) - _ = d.Set("has_projects", repo.GetHasProjects()) - _ = d.Set("has_wiki", repo.GetHasWiki()) - _ = d.Set("is_template", repo.GetIsTemplate()) - _ = d.Set("full_name", repo.GetFullName()) - _ = d.Set("default_branch", repo.GetDefaultBranch()) - _ = d.Set("html_url", repo.GetHTMLURL()) - _ = d.Set("ssh_clone_url", repo.GetSSHURL()) - _ = d.Set("svn_url", repo.GetSVNURL()) - _ = d.Set("git_clone_url", repo.GetGitURL()) - _ = d.Set("http_clone_url", repo.GetCloneURL()) - _ = d.Set("archived", repo.GetArchived()) - _ = d.Set("topics", flattenStringList(repo.Topics)) - _ = d.Set("node_id", repo.GetNodeID()) - _ = d.Set("repo_id", repo.GetID()) - - if err := errors.Join( - d.Set("allow_auto_merge", repo.GetAllowAutoMerge()), - d.Set("allow_merge_commit", repo.GetAllowMergeCommit()), - d.Set("allow_rebase_merge", repo.GetAllowRebaseMerge()), - d.Set("allow_squash_merge", repo.GetAllowSquashMerge()), - d.Set("allow_update_branch", repo.GetAllowUpdateBranch()), - d.Set("allow_forking", repo.GetAllowForking()), - d.Set("delete_branch_on_merge", repo.GetDeleteBranchOnMerge()), - d.Set("web_commit_signoff_required", repo.GetWebCommitSignoffRequired()), - d.Set("has_downloads", repo.GetHasDownloads()), - d.Set("merge_commit_message", repo.GetMergeCommitMessage()), - d.Set("merge_commit_title", repo.GetMergeCommitTitle()), - d.Set("squash_merge_commit_message", repo.GetSquashMergeCommitMessage()), - d.Set("squash_merge_commit_title", repo.GetSquashMergeCommitTitle()), - ); err != nil { - tflog.Warn(ctx, "Error setting repository fields, which were previously ignored for archived repositories", map[string]any{"error": err.Error()}) - } - - if repo.GetHasPages() { - pages, _, err := client.Repositories.GetPagesInfo(ctx, owner, repoName) - if err != nil { - return diag.FromErr(err) - } - if err := d.Set("pages", flattenPages(pages)); err != nil { - return diag.Errorf("error setting pages: %s", err.Error()) - } - } - - // Set fork information if this is a fork - if repo.GetFork() { - _ = d.Set("fork", "true") - - // If the repository has parent information, set the source details - if repo.Parent != nil { - _ = d.Set("source_owner", repo.Parent.GetOwner().GetLogin()) - _ = d.Set("source_repo", repo.Parent.GetName()) - } - } else { - _ = d.Set("fork", "false") - _ = d.Set("source_owner", "") - _ = d.Set("source_repo", "") - } - - if repo.TemplateRepository != nil { - if err = d.Set("template", []any{ - map[string]any{ - "owner": repo.TemplateRepository.Owner.Login, - "repository": repo.TemplateRepository.Name, - }, - }); err != nil { - return diag.FromErr(err) - } - } else { - if err = d.Set("template", []any{}); err != nil { - return diag.FromErr(err) - } + if err := d.Set("etag", resp.Header.Get("ETag")); err != nil { + return diag.FromErr(err) } - if repo.GetSecurityAndAnalysis() != nil { - vulnerabilityAlerts, _, err := client.Repositories.GetVulnerabilityAlerts(ctx, owner, repoName) - if err != nil { - return diag.Errorf("error reading repository vulnerability alerts: %s", err.Error()) - } - if err = d.Set("vulnerability_alerts", vulnerabilityAlerts); err != nil { - return diag.FromErr(err) - } - - if err = d.Set("security_and_analysis", flattenSecurityAndAnalysis(repo.SecurityAndAnalysis)); err != nil { - return diag.FromErr(err) - } + if diags := setRepositoryFieldsFromRepo(ctx, d, client, owner, repo); diags.HasError() { + return diags } return nil @@ -952,7 +904,6 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, repoName := d.Id() owner := meta.(*Owner).name - ctx = context.WithValue(ctx, ctxId, d.Id()) repo, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) if err != nil { @@ -1009,22 +960,26 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, repoReq.AllowForking = allowForking tflog.Debug(ctx, "Updating repository visibility", map[string]any{"from": repo.GetVisibility(), "to": visibility}) - _, resp, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) + updatedRepo, resp, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) if err != nil { if resp.StatusCode != 422 || !strings.Contains(err.Error(), fmt.Sprintf("Visibility is already %s", visibility)) { return diag.FromErr(err) } } + repo = updatedRepo + } + + if diags := setRepositoryFieldsFromRepo(ctx, d, client, owner, repo); diags.HasError() { + return diags } - return resourceGithubRepositoryRead(ctx, d, meta) + return nil } func resourceGithubRepositoryDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client repoName := d.Id() owner := meta.(*Owner).name - ctx = context.WithValue(ctx, ctxId, d.Id()) archiveOnDestroy := d.Get("archive_on_destroy").(bool) if archiveOnDestroy { @@ -1054,6 +1009,120 @@ func resourceGithubRepositoryImport(ctx context.Context, d *schema.ResourceData, return []*schema.ResourceData{d}, nil } +// setRepositoryFieldsFromRepo populates all Computed and Optional+Computed schema fields +// from a *github.Repository API response. Used by Create and Update to set state directly +// instead of chaining to Read. For fields requiring separate API calls (pages, vulnerability +// alerts), targeted calls are made here. +func setRepositoryFieldsFromRepo(ctx context.Context, d *schema.ResourceData, client *github.Client, owner string, repo *github.Repository) diag.Diagnostics { + repoName := repo.GetName() + + if err := errors.Join( + d.Set("name", repoName), + d.Set("description", repo.GetDescription()), + d.Set("primary_language", repo.GetLanguage()), + d.Set("homepage_url", repo.GetHomepage()), + d.Set("private", repo.GetPrivate()), + d.Set("visibility", repo.GetVisibility()), + d.Set("has_issues", repo.GetHasIssues()), + d.Set("has_discussions", repo.GetHasDiscussions()), + d.Set("has_projects", repo.GetHasProjects()), + d.Set("has_wiki", repo.GetHasWiki()), + d.Set("is_template", repo.GetIsTemplate()), + d.Set("full_name", repo.GetFullName()), + d.Set("default_branch", repo.GetDefaultBranch()), + d.Set("html_url", repo.GetHTMLURL()), + d.Set("ssh_clone_url", repo.GetSSHURL()), + d.Set("svn_url", repo.GetSVNURL()), + d.Set("git_clone_url", repo.GetGitURL()), + d.Set("http_clone_url", repo.GetCloneURL()), + d.Set("archived", repo.GetArchived()), + d.Set("topics", flattenStringList(repo.Topics)), + d.Set("node_id", repo.GetNodeID()), + d.Set("repo_id", repo.GetID()), + d.Set("allow_auto_merge", repo.GetAllowAutoMerge()), + d.Set("allow_merge_commit", repo.GetAllowMergeCommit()), + d.Set("allow_rebase_merge", repo.GetAllowRebaseMerge()), + d.Set("allow_squash_merge", repo.GetAllowSquashMerge()), + d.Set("allow_update_branch", repo.GetAllowUpdateBranch()), + d.Set("allow_forking", repo.GetAllowForking()), + d.Set("delete_branch_on_merge", repo.GetDeleteBranchOnMerge()), + d.Set("web_commit_signoff_required", repo.GetWebCommitSignoffRequired()), + d.Set("has_downloads", repo.GetHasDownloads()), + d.Set("merge_commit_message", repo.GetMergeCommitMessage()), + d.Set("merge_commit_title", repo.GetMergeCommitTitle()), + d.Set("squash_merge_commit_message", repo.GetSquashMergeCommitMessage()), + d.Set("squash_merge_commit_title", repo.GetSquashMergeCommitTitle()), + ); err != nil { + return diag.FromErr(err) + } + + // Fork info + if repo.GetFork() { + if err := d.Set("fork", "true"); err != nil { + return diag.FromErr(err) + } + if repo.Parent != nil { + if err := errors.Join( + d.Set("source_owner", repo.Parent.GetOwner().GetLogin()), + d.Set("source_repo", repo.Parent.GetName()), + ); err != nil { + return diag.FromErr(err) + } + } + } else { + if err := errors.Join( + d.Set("fork", "false"), + d.Set("source_owner", ""), + d.Set("source_repo", ""), + ); err != nil { + return diag.FromErr(err) + } + } + + // Template info + if repo.TemplateRepository != nil { + if err := d.Set("template", []any{ + map[string]any{ + "owner": repo.TemplateRepository.Owner.Login, + "repository": repo.TemplateRepository.Name, + }, + }); err != nil { + return diag.FromErr(err) + } + } else { + if err := d.Set("template", []any{}); err != nil { + return diag.FromErr(err) + } + } + + // Pages (requires separate API call for computed fields like url, status) + if repo.GetHasPages() { + pages, _, err := client.Repositories.GetPagesInfo(ctx, owner, repoName) + if err != nil { + return diag.FromErr(err) + } + if err := d.Set("pages", flattenPages(pages)); err != nil { + return diag.FromErr(err) + } + } + + // Security and analysis + vulnerability alerts (requires separate API call) + if repo.GetSecurityAndAnalysis() != nil { + vulnerabilityAlerts, _, err := client.Repositories.GetVulnerabilityAlerts(ctx, owner, repoName) + if err != nil { + return diag.Errorf("error reading repository vulnerability alerts: %s", err.Error()) + } + if err := errors.Join( + d.Set("vulnerability_alerts", vulnerabilityAlerts), + d.Set("security_and_analysis", flattenSecurityAndAnalysis(repo.SecurityAndAnalysis)), + ); err != nil { + return diag.FromErr(err) + } + } + + return nil +} + func expandPages(input []any) *github.Pages { if len(input) == 0 || input[0] == nil { return nil From 7c3c2b0c09b89511b7c117fa68d28d14e637e8ed Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 28 Mar 2026 17:41:00 +0200 Subject: [PATCH 05/15] Enable parallel testing Signed-off-by: Timo Sand --- GNUmakefile | 2 +- github/resource_github_repository_test.go | 86 +++++++++++------------ 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/GNUmakefile b/GNUmakefile index 0409d0b9ed..47580a4c2f 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -59,7 +59,7 @@ test: testacc: @branch=$$(git rev-parse --abbrev-ref HEAD); \ printf "==> Running acceptance tests on branch: \033[1m%s\033[0m...\n" "🌿 $$branch 🌿" - TF_ACC=1 CGO_ENABLED=0 go test $(TEST) -v -run '^TestAcc' $(RUNARGS) $(TESTARGS) -timeout 120m -count=1 + TF_ACC=1 CGO_ENABLED=0 go test $(TEST) -parallel=4 -v -run '^TestAcc' $(RUNARGS) $(TESTARGS) -timeout 120m -count=1 sweep: @echo "WARNING: This will destroy infrastructure. Use only in development accounts." diff --git a/github/resource_github_repository_test.go b/github/resource_github_repository_test.go index 4864019760..23dd5f4916 100644 --- a/github/resource_github_repository_test.go +++ b/github/resource_github_repository_test.go @@ -41,7 +41,7 @@ func TestAccGithubRepository(t *testing.T) { } `, testRepoName, testAccConf.testRepositoryVisibility) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -72,7 +72,7 @@ func TestAccGithubRepository(t *testing.T) { } `, oldName, randomID, testAccConf.testRepositoryVisibility) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -110,7 +110,7 @@ func TestAccGithubRepository(t *testing.T) { } `, testRepoName, testAccConf.testRepositoryVisibility) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -124,7 +124,7 @@ func TestAccGithubRepository(t *testing.T) { ResourceName: "github_repository.test", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"auto_init", "vulnerability_alerts", "ignore_vulnerability_alerts_during_read"}, + ImportStateVerifyIgnore: []string{"auto_init", "vulnerability_alerts", "ignore_vulnerability_alerts_during_read", "etag"}, }, }, }) @@ -142,7 +142,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -174,7 +174,7 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -214,7 +214,7 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -263,7 +263,7 @@ resource "github_repository" "test" { statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("default_branch"), knownvalue.StringExact("main")), } - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -297,7 +297,7 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -326,7 +326,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -362,7 +362,7 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility, testAccConf.testPublicTemplateRepositoryOwner, testAccConf.testPublicTemplateRepository) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t); skipIfEMUEnterprise(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -392,7 +392,7 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility, testAccConf.owner, testAccConf.testOrgTemplateRepository) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -420,7 +420,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -453,7 +453,7 @@ resource "github_repository" "test" { } `, repoName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -481,7 +481,7 @@ resource "github_repository" "test" { } `, repoName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -507,7 +507,7 @@ resource "github_repository" "test" { } `, repoName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -548,7 +548,7 @@ resource "github_repository" "test" { } `, testRepoName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) skipIfEMUEnterprise(t) @@ -593,7 +593,7 @@ resource "github_repository" "test" { } `, repoName, testAccConf.testRepositoryVisibility) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -620,7 +620,7 @@ resource "github_repository" "test" { } `, repoName, testAccConf.testRepositoryVisibility) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -647,7 +647,7 @@ resource "github_repository" "test" { // NOTE: terraform-plugin-testing does not support asserting the nonexistence of a value // (no equivalent to the legacy TestCheckNoResourceAttr). We only verify creation succeeds. - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -671,7 +671,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -709,7 +709,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -750,7 +750,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -791,7 +791,7 @@ resource "github_repository" "test" { // resource.TestCheckResourceAttr("github_repository.test", "primary_language", "Go"), // ) - // resource.Test(t, resource.TestCase{ + // resource.ParallelTest(t, resource.TestCase{ // PreCheck: func() { skipUnauthenticated(t) }, // ProviderFactories: providerFactories, // Steps: []resource.TestStep{ @@ -826,7 +826,7 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -859,7 +859,7 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -913,7 +913,7 @@ resource "github_repository" "test" { `, testRepoName) securityPath := tfjsonpath.New("security_and_analysis").AtSliceIndex(0) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -953,7 +953,7 @@ resource "github_repository" "test" { `, testRepoName) securityPath := tfjsonpath.New("security_and_analysis").AtSliceIndex(0) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) skipIfEMUEnterprise(t) @@ -981,7 +981,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1005,7 +1005,7 @@ resource "github_repository" "test" { } `, testRepoName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnlessMode(t, enterprise) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1030,7 +1030,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t); skipIfEMUEnterprise(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1060,7 +1060,7 @@ resource "github_repository" "test" { } `, testRepoName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1093,7 +1093,7 @@ resource "github_repository" "test" { } `, testRepoName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnlessMode(t, enterprise) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1130,7 +1130,7 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testPublicTemplateRepositoryOwner, testAccConf.testPublicTemplateRepository) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t); skipIfEMUEnterprise(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1159,7 +1159,7 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testPublicTemplateRepositoryOwner, testAccConf.testPublicTemplateRepository) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnlessMode(t, enterprise); skipIfEMUEnterprise(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1185,7 +1185,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1210,7 +1210,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1234,7 +1234,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1262,7 +1262,7 @@ resource "github_repository" "test" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1293,7 +1293,7 @@ resource "github_repository" "private" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1324,7 +1324,7 @@ resource "github_repository" "private" { } ` - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1398,7 +1398,7 @@ func Test_expandPages(t *testing.T) { } `, testRepoName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ @@ -1443,7 +1443,7 @@ func Test_expandPages(t *testing.T) { } `, testRepoName) - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ From f756b56c61158a800e7e2f01a8bc789cc5d61ff8 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 28 Mar 2026 21:25:27 +0200 Subject: [PATCH 06/15] Reduce to using a single `d.SetId` in `Create` Signed-off-by: Timo Sand --- github/resource_github_repository.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index b97af9e34d..aafc55c5d1 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -660,6 +660,8 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, owner := meta.(*Owner).name repoName := repoReq.GetName() + var upstreamRepo *github.Repository + if template, ok := d.GetOk("template"); ok { templateConfigBlocks := template.([]any) @@ -693,7 +695,7 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, return diag.FromErr(err) } - d.SetId(repo.GetName()) + upstreamRepo = repo } } else if d.Get("fork").(string) == "true" { // Handle repository forking @@ -734,27 +736,24 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, tflog.Debug(ctx, "Fork response status", map[string]any{"status": resp.StatusCode}) } - if fork == nil { - return diag.Errorf("fork creation failed - no repository returned") - } + upstreamRepo = fork tflog.Info(ctx, "Fork created", map[string]any{"name": fork.GetName()}) - d.SetId(fork.GetName()) } else { // Create without a repository template - var repo *github.Repository + var err error if meta.(*Owner).IsOrganization { - repo, _, err = client.Repositories.Create(ctx, owner, repoReq) + upstreamRepo, _, err = client.Repositories.Create(ctx, owner, repoReq) } else { // Create repository within authenticated user's account - repo, _, err = client.Repositories.Create(ctx, "", repoReq) + upstreamRepo, _, err = client.Repositories.Create(ctx, "", repoReq) } if err != nil { return diag.FromErr(err) } - d.SetId(repo.GetName()) } + d.SetId(upstreamRepo.GetName()) // TODO: To be able to support repos for different owners, this needs to be updated to something like `buildID(owner,repo.GetName()` topics := repoReq.Topics if len(topics) > 0 { From ff0ea274b48269aa3fb50e2b629fa1efd0002fca Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 28 Mar 2026 21:42:51 +0200 Subject: [PATCH 07/15] Remove unnecessary copy-pasta Signed-off-by: Timo Sand --- github/resource_github_repository.go | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index aafc55c5d1..6af8fc264f 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -771,20 +771,7 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, } } - // handle visibility updates separately from other fields - visibility := repoReq.GetVisibility() - repoReq.Visibility = nil - - // This change needs to be made with the correct visibility - allowForking := repoReq.AllowForking - if d.HasChanges("visibility", "private") { - repoReq.AllowForking = nil - } - - if !d.HasChange("security_and_analysis") { - repoReq.SecurityAndAnalysis = nil - } - + tflog.Info(ctx, "Patching repository to ensure all configurations are applied", map[string]any{"owner": owner, "name": repoName}) _, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) if err != nil { return diag.FromErr(err) @@ -798,18 +785,6 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, } } - if d.HasChanges("visibility", "private") { - repoReq.Visibility = new(visibility) - repoReq.AllowForking = allowForking - - _, resp, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) - if err != nil { - if resp.StatusCode != 422 || !strings.Contains(err.Error(), fmt.Sprintf("Visibility is already %s", visibility)) { - return diag.FromErr(err) - } - } - } - // Fetch final repo state after all mutations and set Computed fields finalRepo, _, err := client.Repositories.Get(ctx, owner, d.Id()) if err != nil { From 0217f3ebec2650cafcaf1809cb5c0d7bfdc236d8 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 28 Mar 2026 21:52:16 +0200 Subject: [PATCH 08/15] Minor cleanup Signed-off-by: Timo Sand --- github/resource_github_repository.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index 6af8fc264f..4bf632210a 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -649,15 +649,16 @@ func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository { return repository } -func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*Owner).v3client +func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client if branchName, hasDefaultBranch := d.GetOk("default_branch"); hasDefaultBranch && (branchName != "main") { return diag.Errorf("cannot set the default branch on a new repository to something other than 'main'") } repoReq := resourceGithubRepositoryObject(d) - owner := meta.(*Owner).name + owner := meta.name repoName := repoReq.GetName() var upstreamRepo *github.Repository @@ -713,7 +714,7 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, Name: requestedName, } - if meta.(*Owner).IsOrganization { + if meta.IsOrganization { opts.Organization = owner } @@ -743,12 +744,16 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, // Create without a repository template var err error - if meta.(*Owner).IsOrganization { - upstreamRepo, _, err = client.Repositories.Create(ctx, owner, repoReq) + var repoOwner string + if meta.IsOrganization { + repoOwner = owner } else { - // Create repository within authenticated user's account - upstreamRepo, _, err = client.Repositories.Create(ctx, "", repoReq) + // TODO: This is deprecated since `owner` should already be set to the authenticated user + tflog.Info(ctx, "Creating repository for authenticated user") + repoOwner = "" } + + upstreamRepo, _, err = client.Repositories.Create(ctx, repoOwner, repoReq) if err != nil { return diag.FromErr(err) } @@ -936,7 +941,7 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, tflog.Debug(ctx, "Updating repository visibility", map[string]any{"from": repo.GetVisibility(), "to": visibility}) updatedRepo, resp, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) if err != nil { - if resp.StatusCode != 422 || !strings.Contains(err.Error(), fmt.Sprintf("Visibility is already %s", visibility)) { + if resp.StatusCode != http.StatusUnprocessableEntity || !strings.Contains(err.Error(), fmt.Sprintf("Visibility is already %s", visibility)) { return diag.FromErr(err) } } From 17455f926df401eb0313fda87ebbe42abaf6a361 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 29 Mar 2026 10:05:25 +0300 Subject: [PATCH 09/15] Add missing field to repo DS Signed-off-by: Timo Sand --- github/data_source_github_repository.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/github/data_source_github_repository.go b/github/data_source_github_repository.go index 8fd515c74b..87d6b0dd2f 100644 --- a/github/data_source_github_repository.go +++ b/github/data_source_github_repository.go @@ -341,6 +341,11 @@ func dataSourceGithubRepository() *schema.Resource { Type: schema.TypeBool, Computed: true, }, + "web_commit_signoff_required": { + Type: schema.TypeBool, + Computed: true, + Description: "Require contributors to sign off on web-based commits.", + }, }, } } @@ -414,6 +419,7 @@ func dataSourceGithubRepositoryRead(ctx context.Context, d *schema.ResourceData, _ = d.Set("has_projects", repo.GetHasProjects()) _ = d.Set("delete_branch_on_merge", repo.GetDeleteBranchOnMerge()) _ = d.Set("allow_update_branch", repo.GetAllowUpdateBranch()) + _ = d.Set("web_commit_signoff_required", repo.GetWebCommitSignoffRequired()) if repo.GetHasPages() { pages, _, err := client.Repositories.GetPagesInfo(ctx, owner, repoName) From edbc06ae3999c3220dbf026d884ab50dedb3ec53 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 29 Mar 2026 11:42:59 +0300 Subject: [PATCH 10/15] Update docs Signed-off-by: Timo Sand --- website/docs/r/repository.html.markdown | 151 ++++++++++++------------ 1 file changed, 78 insertions(+), 73 deletions(-) diff --git a/website/docs/r/repository.html.markdown b/website/docs/r/repository.html.markdown index 157fd01caa..89a1f1ce7d 100644 --- a/website/docs/r/repository.html.markdown +++ b/website/docs/r/repository.html.markdown @@ -36,7 +36,7 @@ resource "github_repository" "example" { name = "example" description = "My awesome web page" - private = false + visibility = public pages { source { @@ -63,156 +63,156 @@ resource "github_repository" "forked_repo" { The following arguments are supported: -* `name` - (Required) The name of the repository. +- `name` - (Required) The name of the repository. -* `description` - (Optional) A description of the repository. +- `description` - (Optional) A description of the repository. -* `homepage_url` - (Optional) URL of a page describing the project. +- `homepage_url` - (Optional) URL of a page describing the project. -* `fork` - (Optional) Set to `true` to create a fork of an existing repository. When set to `true`, both `source_owner` and `source_repo` must also be specified. +- `fork` - (Optional) Set to `true` to create a fork of an existing repository. When set to `true`, both `source_owner` and `source_repo` must also be specified. -* `source_owner` - (Optional) The GitHub username or organization that owns the repository being forked. Required when `fork` is `true`. +- `source_owner` - (Optional) The GitHub username or organization that owns the repository being forked. Required when `fork` is `true`. -* `source_repo` - (Optional) The name of the repository to fork. Required when `fork` is `true`. +- `source_repo` - (Optional) The name of the repository to fork. Required when `fork` is `true`. -* `private` - (Optional) Set to `true` to create a private repository. - Repositories are created as public (e.g. open source) by default. +- `private` - (**DEPRECATED**) (Optional) Set to `true` to create a private repository. + Repositories are created as public (e.g. open source) by default. Use `visibility` instead. -* `visibility` - (Optional) Can be `public` or `private`. If your organization is associated with an enterprise account using GitHub Enterprise Cloud or GitHub Enterprise Server 2.20+, visibility can also be `internal`. The `visibility` parameter overrides the `private` parameter. +- `visibility` - (Optional) Can be `public` or `private`. If your organization is associated with an enterprise account using GitHub Enterprise Cloud or GitHub Enterprise Server 2.20+, visibility can also be `internal`. The `visibility` parameter overrides the `private` parameter. -* `has_issues` - (Optional) Set to `true` to enable the GitHub Issues features +- `has_issues` - (Optional) Set to `true` to enable the GitHub Issues features on the repository. -* `has_discussions` - (Optional) Set to `true` to enable GitHub Discussions on the repository. Defaults to `false`. +- `has_discussions` - (Optional) Set to `true` to enable GitHub Discussions on the repository. Defaults to `false`. -* `has_projects` - (Optional) Set to `true` to enable the GitHub Projects features on the repository. Per the GitHub [documentation](https://developer.github.com/v3/repos/#create) when in an organization that has disabled repository projects it will default to `false` and will otherwise default to `true`. If you specify `true` when it has been disabled it will return an error. +- `has_projects` - (Optional) Set to `true` to enable the GitHub Projects features on the repository. Per the GitHub [documentation](https://developer.github.com/v3/repos/#create) when in an organization that has disabled repository projects it will default to `false` and will otherwise default to `true`. If you specify `true` when it has been disabled it will return an error. -* `has_wiki` - (Optional) Set to `true` to enable the GitHub Wiki features on +- `has_wiki` - (Optional) Set to `true` to enable the GitHub Wiki features on the repository. -* `is_template` - (Optional) Set to `true` to tell GitHub that this is a template repository. +- `is_template` - (Optional) Set to `true` to tell GitHub that this is a template repository. -* `allow_merge_commit` - (Optional) Set to `false` to disable merge commits on the repository. +- `allow_merge_commit` - (Optional) Set to `false` to disable merge commits on the repository. -* `allow_squash_merge` - (Optional) Set to `false` to disable squash merges on the repository. +- `allow_squash_merge` - (Optional) Set to `false` to disable squash merges on the repository. -* `allow_rebase_merge` - (Optional) Set to `false` to disable rebase merges on the repository. +- `allow_rebase_merge` - (Optional) Set to `false` to disable rebase merges on the repository. -* `allow_auto_merge` - (Optional) Set to `true` to allow auto-merging pull requests on the repository. +- `allow_auto_merge` - (Optional) Set to `true` to allow auto-merging pull requests on the repository. -* `allow_forking` - (Optional) Configure private forking for organization owned private and internal repositories; set to `true` to enable, `false` to disable, and leave unset for the default behaviour. Configuring this requires that private forking is not being explicitly configured at the organization level. +- `allow_forking` - (Optional) Configure private forking for organization owned private and internal repositories; set to `true` to enable, `false` to disable, and leave unset for the default behaviour. Configuring this requires that private forking is not being explicitly configured at the organization level. -* `squash_merge_commit_title` - (Optional) Can be `PR_TITLE` or `COMMIT_OR_PR_TITLE` for a default squash merge commit title. Applicable only if `allow_squash_merge` is `true`. +- `squash_merge_commit_title` - (Optional) Can be `PR_TITLE` or `COMMIT_OR_PR_TITLE` for a default squash merge commit title. Applicable only if `allow_squash_merge` is `true`. -* `squash_merge_commit_message` - (Optional) Can be `PR_BODY`, `COMMIT_MESSAGES`, or `BLANK` for a default squash merge commit message. Applicable only if `allow_squash_merge` is `true`. +- `squash_merge_commit_message` - (Optional) Can be `PR_BODY`, `COMMIT_MESSAGES`, or `BLANK` for a default squash merge commit message. Applicable only if `allow_squash_merge` is `true`. -* `merge_commit_title` - Can be `PR_TITLE` or `MERGE_MESSAGE` for a default merge commit title. Applicable only if `allow_merge_commit` is `true`. +- `merge_commit_title` - (Optional) Can be `PR_TITLE` or `MERGE_MESSAGE` for a default merge commit title. Applicable only if `allow_merge_commit` is `true`. -* `merge_commit_message` - Can be `PR_BODY`, `PR_TITLE`, or `BLANK` for a default merge commit message. Applicable only if `allow_merge_commit` is `true`. +- `merge_commit_message` - (Optional) Can be `PR_BODY`, `PR_TITLE`, or `BLANK` for a default merge commit message. Applicable only if `allow_merge_commit` is `true`. -* `delete_branch_on_merge` - (Optional) Automatically delete head branch after a pull request is merged. Defaults to `false`. +- `delete_branch_on_merge` - (Optional) Automatically delete head branch after a pull request is merged. Defaults to `false`. -* `web_commit_signoff_required` - (Optional) Require contributors to sign off on web-based commits. See more [here](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/managing-the-commit-signoff-policy-for-your-repository). +- `web_commit_signoff_required` - (Optional) Require contributors to sign off on web-based commits. See more in the [GitHub documentation about managing the commit signoff policy for your repository](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/managing-the-commit-signoff-policy-for-your-repository). -* `has_downloads` - (**DEPRECATED**) (Optional) Set to `true` to enable the (deprecated) downloads features on the repository. This attribute is no longer in use, but it hasn't been removed yet. It will be removed in a future version. See [this discussion](https://github.com/orgs/community/discussions/102145#discussioncomment-8351756). +- `has_downloads` - (**DEPRECATED**) (Optional) Set to `true` to enable the (deprecated) downloads features on the repository. This attribute is no longer in use, but it hasn't been removed yet. It will be removed in a future version. See [this discussion](https://github.com/orgs/community/discussions/102145#discussioncomment-8351756). -* `auto_init` - (Optional) Set to `true` to produce an initial commit in the repository. +- `auto_init` - (Optional) Set to `true` to produce an initial commit in the repository. -* `gitignore_template` - (Optional) Use the [name of the template](https://github.com/github/gitignore) without the extension. For example, "Haskell". +- `gitignore_template` - (Optional) Use the [name of the template](https://github.com/github/gitignore) without the extension. For example, "Haskell". -* `license_template` - (Optional) Use the [name of the template](https://github.com/github/choosealicense.com/tree/gh-pages/_licenses) without the extension. For example, "mit" or "mpl-2.0". +- `license_template` - (Optional) Use the [name of the template](https://github.com/github/choosealicense.com/tree/gh-pages/_licenses) without the extension. For example, "mit" or "mpl-2.0". -* `default_branch` - (Optional) (Deprecated: Use `github_branch_default` resource instead) The name of the default branch of the repository. **NOTE:** This can only be set after a repository has already been created, +- `default_branch` - (Optional) (Deprecated: Use `github_branch_default` resource instead) The name of the default branch of the repository. **NOTE:** This can only be set after a repository has already been created, and after a correct reference has been created for the target branch inside the repository. This means a user will have to omit this parameter from the initial repository creation and create the target branch inside of the repository prior to setting this attribute. -* `archived` - (Optional) Specifies if the repository should be archived. Defaults to `false`. **NOTE** Currently, the API does not support unarchiving. +- `archived` - (Optional) Specifies if the repository should be archived. Defaults to `false`. **NOTE** Currently, the API does not support unarchiving. -* `archive_on_destroy` - (Optional) Set to `true` to archive the repository instead of deleting on destroy. +- `archive_on_destroy` - (Optional) Set to `true` to archive the repository instead of deleting on destroy. -* `pages` - (Optional) The repository's GitHub Pages configuration. See [GitHub Pages Configuration](#github-pages-configuration) below for details. +- `pages` - (Optional) The repository's GitHub Pages configuration. See [GitHub Pages Configuration](#github-pages-configuration) below for details. -* `security_and_analysis` - (Optional) The repository's [security and analysis](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-security-and-analysis-settings-for-your-repository) configuration. See [Security and Analysis Configuration](#security-and-analysis-configuration) below for details. +- `security_and_analysis` - (Optional) The repository's [security and analysis](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-security-and-analysis-settings-for-your-repository) configuration. See [Security and Analysis Configuration](#security-and-analysis-configuration) below for details. -* `topics` - (Optional) The list of topics of the repository. +- `topics` - (Optional) The list of topics of the repository. ~> Note: This attribute is not compatible with the `github_repository_topics` resource. Use one of them. `github_repository_topics` is only meant to be used if the repository itself is not handled via terraform, for example if it's only read as a datasource (see [issue #1845](https://github.com/integrations/terraform-provider-github/issues/1845)). -* `template` - (Optional) Use a template repository to create this resource. See [Template Repositories](#template-repositories) below for details. +- `template` - (Optional) Use a template repository to create this resource. See [Template Repositories](#template-repositories) below for details. -* `vulnerability_alerts` - (Optional) Configure [Dependabot security alerts](https://help.github.com/en/github/managing-security-vulnerabilities/about-security-alerts-for-vulnerable-dependencies) for vulnerable dependencies; set to `true` to enable, set to `false` to disable, and leave unset for the default behavior. Configuring this requires that alerts are not being explicitly configured at the organization level. +- `vulnerability_alerts` - (Optional) Configure [Dependabot security alerts](https://help.github.com/en/github/managing-security-vulnerabilities/about-security-alerts-for-vulnerable-dependencies) for vulnerable dependencies; set to `true` to enable, set to `false` to disable, and leave unset for the default behavior. Configuring this requires that alerts are not being explicitly configured at the organization level. -* `ignore_vulnerability_alerts_during_read` (**DEPRECATED**) (Optional) - This is ignored as the provider now handles lack of permissions automatically. +- `ignore_vulnerability_alerts_during_read` (**DEPRECATED**) (Optional) - This is ignored as the provider now handles lack of permissions automatically. -* `allow_update_branch` (Optional) - Set to `true` to always suggest updating pull request branches. +- `allow_update_branch` (Optional) - Set to `true` to always suggest updating pull request branches. ### GitHub Pages Configuration The `pages` block supports the following: -* `source` - (Optional) The source branch and directory for the rendered Pages site. See [GitHub Pages Source](#github-pages-source) below for details. +- `source` - (Optional) The source branch and directory for the rendered Pages site. See [GitHub Pages Source](#github-pages-source) below for details. -* `build_type` - (Optional) The type of GitHub Pages site to build. Can be `legacy` or `workflow`. If you use `legacy` as build type you need to set the option `source`. +- `build_type` - (Optional) The type of GitHub Pages site to build. Can be `legacy` or `workflow`. If you use `legacy` as build type you need to set the option `source`. -* `cname` - (Optional) The custom domain for the repository. This can only be set after the repository has been created. +- `cname` - (Optional) The custom domain for the repository. This can only be set after the repository has been created. #### GitHub Pages Source The `source` block supports the following: -* `branch` - (Required) The repository branch used to publish the site's source files. (i.e. `main` or `gh-pages`. +- `branch` - (Required) The repository branch used to publish the site's source files. (i.e. `main` or `gh-pages`. -* `path` - (Optional) The repository directory from which the site publishes (Default: `/`). +- `path` - (Optional) The repository directory from which the site publishes (Default: `/`). ### Security and Analysis Configuration The `security_and_analysis` block supports the following: -* `advanced_security` - (Optional) The advanced security configuration for the repository. See [Advanced Security Configuration](#advanced-security-configuration) below for details. If a repository's visibility is `public`, advanced security is always enabled and cannot be changed, so this setting cannot be supplied. +- `advanced_security` - (Optional) The advanced security configuration for the repository. See [Advanced Security Configuration](#advanced-security-configuration) below for details. If a repository's visibility is `public`, advanced security is always enabled and cannot be changed, so this setting cannot be supplied. -* `code_security` - (Optional) The code security configuration for the repository. See [Code Security](#code-security-configuration) below for details. +- `code_security` - (Optional) The code security configuration for the repository. See [Code Security](#code-security-configuration) below for details. -* `secret_scanning` - (Optional) The secret scanning configuration for the repository. See [Secret Scanning Configuration](#secret-scanning-configuration) below for details. +- `secret_scanning` - (Optional) The secret scanning configuration for the repository. See [Secret Scanning Configuration](#secret-scanning-configuration) below for details. -* `secret_scanning_push_protection` - (Optional) The secret scanning push protection configuration for the repository. See [Secret Scanning Push Protection Configuration](#secret-scanning-push-protection-configuration) below for details. +- `secret_scanning_push_protection` - (Optional) The secret scanning push protection configuration for the repository. See [Secret Scanning Push Protection Configuration](#secret-scanning-push-protection-configuration) below for details. -* `secret_scanning_ai_detection` - (Optional) The secret scanning ai detection configuration for the repository. See [Secret Scanning AI Detection Configuration](#secret-scanning-ai-detection-configuration) below for details. +- `secret_scanning_ai_detection` - (Optional) The secret scanning ai detection configuration for the repository. See [Secret Scanning AI Detection Configuration](#secret-scanning-ai-detection) below for details. -* `secret_scanning_non_provider_patterns` - (Optional) The secret scanning non-provider patterns configuration for this repository. See [Secret Scanning Non-Provider Patterns Configuration](#secret-scanning-non-provider-patterns-configuration) below for more details. +- `secret_scanning_non_provider_patterns` - (Optional) The secret scanning non-provider patterns configuration for this repository. See [Secret Scanning Non-Provider Patterns Configuration](#secret-scanning-non-provider-patterns) below for more details. #### Advanced Security Configuration The `advanced_security` block supports the following: -* `status` - (Required) Set to `enabled` to enable advanced security features on the repository. Can be `enabled` or `disabled`. +- `status` - (Required) Set to `enabled` to enable advanced security features on the repository. Can be `enabled` or `disabled`. #### Code Security Configuration -* `status` - (Required) Set to `enabled` to enable GitHub Code Security on the repository. Can be `enabled` or `disabled`. If set to `enabled`, the repository's visibility must be `public`, `security_and_analysis[0].advanced_security[0].status` must also be set to `enabled`, or your Organization must have split licensing for Advanced security. +- `status` - (Required) Set to `enabled` to enable GitHub Code Security on the repository. Can be `enabled` or `disabled`. If set to `enabled`, the repository's visibility must be `public`, `security_and_analysis[0].advanced_security[0].status` must also be set to `enabled`, or your Organization must have split licensing for Advanced security. #### Secret Scanning Configuration -* `status` - (Required) Set to `enabled` to enable secret scanning on the repository. Can be `enabled` or `disabled`. If set to `enabled`, the repository's visibility must be `public`, `security_and_analysis[0].advanced_security[0].status` must also be set to `enabled`, or your Organization must have split licensing for Advanced security. +- `status` - (Required) Set to `enabled` to enable secret scanning on the repository. Can be `enabled` or `disabled`. If set to `enabled`, the repository's visibility must be `public`, `security_and_analysis[0].advanced_security[0].status` must also be set to `enabled`, or your Organization must have split licensing for Advanced security. #### Secret Scanning Push Protection Configuration -* `status` - (Required) Set to `enabled` to enable secret scanning push protection on the repository. Can be `enabled` or `disabled`. If set to `enabled`, the repository's visibility must be `public`, `security_and_analysis[0].advanced_security[0].status` must also be set to `enabled`, or your Organization must have split licensing for Advanced security. +- `status` - (Required) Set to `enabled` to enable secret scanning push protection on the repository. Can be `enabled` or `disabled`. If set to `enabled`, the repository's visibility must be `public`, `security_and_analysis[0].advanced_security[0].status` must also be set to `enabled`, or your Organization must have split licensing for Advanced security. #### Secret Scanning AI Detection -* `status` - (Required) Set to `enabled` to enable secret scanning AI detection on the repository. Can be `enabled` or `disabled`. If set to `enabled`, the repository's visibility must be `public`, `security_and_analysis[0].advanced_security[0].status` must also be set to `enabled`, or your Organization must have split licensing for Advanced security. +- `status` - (Required) Set to `enabled` to enable secret scanning AI detection on the repository. Can be `enabled` or `disabled`. If set to `enabled`, the repository's visibility must be `public`, `security_and_analysis[0].advanced_security[0].status` must also be set to `enabled`, or your Organization must have split licensing for Advanced security. #### Secret Scanning Non-Provider Patterns -* `status` - (Required) Set to `enabled` to enable secret scanning non-provider patterns on the repository. Can be `enabled` or `disabled`. If set to `enabled`, the repository's visibility must be `public`, `security_and_analysis[0].advanced_security[0].status` must also be set to `enabled`, or your Organization must have split licensing for Advanced security. +- `status` - (Required) Set to `enabled` to enable secret scanning non-provider patterns on the repository. Can be `enabled` or `disabled`. If set to `enabled`, the repository's visibility must be `public`, `security_and_analysis[0].advanced_security[0].status` must also be set to `enabled`, or your Organization must have split licensing for Advanced security. ### Template Repositories `template` supports the following arguments: -* `owner`: The GitHub organization or user the template repository is owned by. -* `repository`: The name of the template repository. -* `include_all_branches`: Whether the new repository should include all the branches from the template repository (defaults to false, which includes only the default branch from the template). +- `owner`: The GitHub organization or user the template repository is owned by. +- `repository`: The name of the template repository. +- `include_all_branches`: Whether the new repository should include all the branches from the template repository (defaults to false, which includes only the default branch from the template). ~> **Note on `internal` visibility with templates**: When creating a repository from a template with `visibility = "internal"`, the provider uses a two-step process due to GitHub API limitations. The template creation API only supports a `private` boolean parameter. Therefore, repositories with `visibility = "internal"` are initially created as private and then immediately updated to internal visibility. This ensures internal repositories are never exposed publicly during creation. @@ -220,28 +220,33 @@ The `advanced_security` block supports the following: The following additional attributes are exported: -* `full_name` - A string of the form "orgname/reponame". +- `full_name` - A string of the form "orgname/reponame". -* `html_url` - URL to the repository on the web. +- `html_url` - URL to the repository on the web. -* `ssh_clone_url` - URL that can be provided to `git clone` to clone the repository via SSH. +- `ssh_clone_url` - URL that can be provided to `git clone` to clone the repository via SSH. -* `http_clone_url` - URL that can be provided to `git clone` to clone the repository via HTTPS. +- `http_clone_url` - URL that can be provided to `git clone` to clone the repository via HTTPS. -* `git_clone_url` - URL that can be provided to `git clone` to clone the repository anonymously via the git protocol. +- `git_clone_url` - URL that can be provided to `git clone` to clone the repository anonymously via the git protocol. -* `svn_url` - URL that can be provided to `svn checkout` to check out the repository via GitHub's Subversion protocol emulation. +- `svn_url` - URL that can be provided to `svn checkout` to check out the repository via GitHub's Subversion protocol emulation. -* `node_id` - GraphQL global node id for use with v4 API +- `node_id` - GraphQL global node id for use with v4 API -* `repo_id` - GitHub ID for the repository +- `repo_id` - GitHub ID for the repository -* `primary_language` - The primary language used in the repository. +- `primary_language` - The primary language used in the repository. -* `pages` - The block consisting of the repository's GitHub Pages configuration with the following additional attributes: -* `custom_404` - Whether the rendered GitHub Pages site has a custom 404 page. -* `html_url` - The absolute URL (including scheme) of the rendered GitHub Pages site e.g. `https://username.github.io`. -* `status` - The GitHub Pages site's build status e.g. `building` or `built`. +- `pages` - The block consisting of the repository's GitHub Pages configuration with the following additional attributes: + +- `custom_404` - Whether the rendered GitHub Pages site has a custom 404 page. + +- `html_url` - The absolute URL (including scheme) of the rendered GitHub Pages site e.g. `https://username.github.io`. + +- `status` - The GitHub Pages site's build status e.g. `building` or `built`. + +- `url` - The absolute URL (including scheme) of the rendered GitHub Pages site e.g. `https://username.github.io`. ## Import From c59191ad4e1af55dccd013205d00d096635fa940 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 29 Mar 2026 21:08:54 +0300 Subject: [PATCH 11/15] Add validation for merge and squash merge configs Signed-off-by: Timo Sand --- github/resource_github_repository.go | 53 +++++++++++++------- github/resource_github_repository_test.go | 60 +++++++++++++++++++++++ 2 files changed, 96 insertions(+), 17 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index 4bf632210a..80a715beae 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -255,28 +255,32 @@ func resourceGithubRepository() *schema.Resource { Description: "Set to 'true' to allow private forking on the repository; this is only relevant if the repository is owned by an organization and is private or internal.", }, "squash_merge_commit_title": { - Type: schema.TypeString, - Optional: true, - Default: "COMMIT_OR_PR_TITLE", - Description: "Can be 'PR_TITLE' or 'COMMIT_OR_PR_TITLE' for a default squash merge commit title.", + Type: schema.TypeString, + Optional: true, + Default: "COMMIT_OR_PR_TITLE", + Description: "Can be 'PR_TITLE' or 'COMMIT_OR_PR_TITLE' for a default squash merge commit title.", + RequiredWith: []string{"allow_squash_merge"}, }, "squash_merge_commit_message": { - Type: schema.TypeString, - Optional: true, - Default: "COMMIT_MESSAGES", - Description: "Can be 'PR_BODY', 'COMMIT_MESSAGES', or 'BLANK' for a default squash merge commit message.", + Type: schema.TypeString, + Optional: true, + Default: "COMMIT_MESSAGES", + Description: "Can be 'PR_BODY', 'COMMIT_MESSAGES', or 'BLANK' for a default squash merge commit message.", + RequiredWith: []string{"allow_squash_merge", "squash_merge_commit_title"}, }, "merge_commit_title": { - Type: schema.TypeString, - Optional: true, - Default: "MERGE_MESSAGE", - Description: "Can be 'PR_TITLE' or 'MERGE_MESSAGE' for a default merge commit title.", + Type: schema.TypeString, + Optional: true, + Default: "MERGE_MESSAGE", + Description: "Can be 'PR_TITLE' or 'MERGE_MESSAGE' for a default merge commit title.", + RequiredWith: []string{"allow_merge_commit"}, }, "merge_commit_message": { - Type: schema.TypeString, - Optional: true, - Default: "PR_TITLE", - Description: "Can be 'PR_BODY', 'PR_TITLE', or 'BLANK' for a default merge commit message.", + Type: schema.TypeString, + Optional: true, + Default: "PR_TITLE", + Description: "Can be 'PR_BODY', 'PR_TITLE', or 'BLANK' for a default merge commit message.", + RequiredWith: []string{"allow_merge_commit", "merge_commit_title"}, }, "delete_branch_on_merge": { Type: schema.TypeBool, @@ -515,6 +519,21 @@ func customDiffFunction(_ context.Context, diff *schema.ResourceDiff, v any) err return err } } + _, titleOk := diff.GetOk("squash_merge_commit_title") + _, messageOk := diff.GetOk("squash_merge_commit_message") + if messageOk && titleOk { + if !diff.Get("allow_squash_merge").(bool) { + return fmt.Errorf("allow_squash_merge is required when squash_merge_commit_title and squash_merge_commit_message is set") + } + } + + _, titleOk = diff.GetOk("merge_commit_title") + _, messageOk = diff.GetOk("merge_commit_message") + if messageOk && titleOk { + if !diff.Get("allow_merge_commit").(bool) { + return fmt.Errorf("allow_merge_commit is required when merge_commit_title and merge_commit_message is set") + } + } return nil } @@ -748,7 +767,7 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, if meta.IsOrganization { repoOwner = owner } else { - // TODO: This is deprecated since `owner` should already be set to the authenticated user + // Note: This exists due to the `go-github` library, which chooses endpoints based on the owner being empty or not tflog.Info(ctx, "Creating repository for authenticated user") repoOwner = "" } diff --git a/github/resource_github_repository_test.go b/github/resource_github_repository_test.go index 23dd5f4916..0c3b5d9e28 100644 --- a/github/resource_github_repository_test.go +++ b/github/resource_github_repository_test.go @@ -731,6 +731,66 @@ resource "github_repository" "test" { }) }) + t.Run("validate_required_fields_for_squash_merge_commit_strategy", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + testRepoName := fmt.Sprintf("%smodify-sq-str-%s", testResourcePrefix, randomID) + + config := ` + resource "github_repository" "test" { + name = "%s" + squash_merge_commit_title = "PR_TITLE" + squash_merge_commit_message = "PR_BODY" + visibility = "%s" + %s + } +` + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, testRepoName, testAccConf.testRepositoryVisibility, "allow_squash_merge = false"), + ExpectError: regexp.MustCompile("allow_squash_merge is required.*"), + }, + { + Config: fmt.Sprintf(config, testRepoName, testAccConf.testRepositoryVisibility, ""), + ExpectError: regexp.MustCompile(`all of[\s\S]*?allow_squash_merge[\s\S]*?must be specified`), + }, + }, + }, + ) + }) + + t.Run("validate_required_fields_for_merge_commit_strategy", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + testRepoName := fmt.Sprintf("%smodify-sq-str-%s", testResourcePrefix, randomID) + + config := ` + resource "github_repository" "test" { + name = "%s" + merge_commit_title = "PR_TITLE" + merge_commit_message = "PR_BODY" + visibility = "%s" + %s + } +` + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, testRepoName, testAccConf.testRepositoryVisibility, "allow_merge_commit = false"), + ExpectError: regexp.MustCompile("allow_merge_commit is required.*"), + }, + { + Config: fmt.Sprintf(config, testRepoName, testAccConf.testRepositoryVisibility, ""), + ExpectError: regexp.MustCompile(`all of[\s\S]*?allow_merge_commit[\s\S]*?must[\s\S]be[\s\S]specified`), + }, + }, + }, + ) + }) + t.Run("create and modify squash merge commit strategy without error", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) testRepoName := fmt.Sprintf("%smodify-sq-str-%s", testResourcePrefix, randomID) From bb1290ac4ccd9d4f4a27408936f5b6183421cacb Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 29 Mar 2026 23:10:29 +0300 Subject: [PATCH 12/15] Refactor to use only `CustomizeDiff` for validation `RequiredWith` doesn't work with `Default` values Signed-off-by: Timo Sand --- github/resource_github_repository.go | 54 +++++++++++------------ github/resource_github_repository_test.go | 12 +++-- 2 files changed, 35 insertions(+), 31 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index 80a715beae..f11dd811b6 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -255,32 +255,28 @@ func resourceGithubRepository() *schema.Resource { Description: "Set to 'true' to allow private forking on the repository; this is only relevant if the repository is owned by an organization and is private or internal.", }, "squash_merge_commit_title": { - Type: schema.TypeString, - Optional: true, - Default: "COMMIT_OR_PR_TITLE", - Description: "Can be 'PR_TITLE' or 'COMMIT_OR_PR_TITLE' for a default squash merge commit title.", - RequiredWith: []string{"allow_squash_merge"}, + Type: schema.TypeString, + Optional: true, + Default: "COMMIT_OR_PR_TITLE", + Description: "Can be 'PR_TITLE' or 'COMMIT_OR_PR_TITLE' for a default squash merge commit title.", }, "squash_merge_commit_message": { - Type: schema.TypeString, - Optional: true, - Default: "COMMIT_MESSAGES", - Description: "Can be 'PR_BODY', 'COMMIT_MESSAGES', or 'BLANK' for a default squash merge commit message.", - RequiredWith: []string{"allow_squash_merge", "squash_merge_commit_title"}, + Type: schema.TypeString, + Optional: true, + Default: "COMMIT_MESSAGES", + Description: "Can be 'PR_BODY', 'COMMIT_MESSAGES', or 'BLANK' for a default squash merge commit message.", }, "merge_commit_title": { - Type: schema.TypeString, - Optional: true, - Default: "MERGE_MESSAGE", - Description: "Can be 'PR_TITLE' or 'MERGE_MESSAGE' for a default merge commit title.", - RequiredWith: []string{"allow_merge_commit"}, + Type: schema.TypeString, + Optional: true, + Default: "MERGE_MESSAGE", + Description: "Can be 'PR_TITLE' or 'MERGE_MESSAGE' for a default merge commit title.", }, "merge_commit_message": { - Type: schema.TypeString, - Optional: true, - Default: "PR_TITLE", - Description: "Can be 'PR_BODY', 'PR_TITLE', or 'BLANK' for a default merge commit message.", - RequiredWith: []string{"allow_merge_commit", "merge_commit_title"}, + Type: schema.TypeString, + Optional: true, + Default: "PR_TITLE", + Description: "Can be 'PR_BODY', 'PR_TITLE', or 'BLANK' for a default merge commit message.", }, "delete_branch_on_merge": { Type: schema.TypeBool, @@ -513,27 +509,31 @@ func valueChangedButNotEmpty(ctx context.Context, oldVal, newVal, meta any) bool return oldValStr != "" && oldValStr != newValStr } -func customDiffFunction(_ context.Context, diff *schema.ResourceDiff, v any) error { +func customDiffFunction(ctx context.Context, diff *schema.ResourceDiff, v any) error { if diff.HasChange("name") { if err := diff.SetNewComputed("full_name"); err != nil { return err } } - _, titleOk := diff.GetOk("squash_merge_commit_title") - _, messageOk := diff.GetOk("squash_merge_commit_message") - if messageOk && titleOk { + + // We need to check the `allow_squash_merge` flag by checking if the `squash_merge_commit_title` and `squash_merge_commit_message` are set in the configuration. + isSquashMergeCommitTitleSet := !diff.GetRawConfig().GetAttr("squash_merge_commit_title").IsNull() + isSquashMergeCommitMessageSet := !diff.GetRawConfig().GetAttr("squash_merge_commit_message").IsNull() + if isSquashMergeCommitMessageSet && isSquashMergeCommitTitleSet { if !diff.Get("allow_squash_merge").(bool) { return fmt.Errorf("allow_squash_merge is required when squash_merge_commit_title and squash_merge_commit_message is set") } } - _, titleOk = diff.GetOk("merge_commit_title") - _, messageOk = diff.GetOk("merge_commit_message") - if messageOk && titleOk { + // We need to check the `allow_merge_commit` flag by checking if the `merge_commit_title` and `merge_commit_message` are set in the configuration. + isMergeCommitTitleSet := !diff.GetRawConfig().GetAttr("merge_commit_title").IsNull() + isMergeCommitMessageSet := !diff.GetRawConfig().GetAttr("merge_commit_message").IsNull() + if isMergeCommitMessageSet && isMergeCommitTitleSet { if !diff.Get("allow_merge_commit").(bool) { return fmt.Errorf("allow_merge_commit is required when merge_commit_title and merge_commit_message is set") } } + return nil } diff --git a/github/resource_github_repository_test.go b/github/resource_github_repository_test.go index 0c3b5d9e28..c38bc29c9a 100644 --- a/github/resource_github_repository_test.go +++ b/github/resource_github_repository_test.go @@ -753,8 +753,10 @@ resource "github_repository" "test" { ExpectError: regexp.MustCompile("allow_squash_merge is required.*"), }, { - Config: fmt.Sprintf(config, testRepoName, testAccConf.testRepositoryVisibility, ""), - ExpectError: regexp.MustCompile(`all of[\s\S]*?allow_squash_merge[\s\S]*?must be specified`), + Config: fmt.Sprintf(config, testRepoName, testAccConf.testRepositoryVisibility, ""), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("allow_squash_merge"), knownvalue.Bool(true)), + }, }, }, }, @@ -783,8 +785,10 @@ resource "github_repository" "test" { ExpectError: regexp.MustCompile("allow_merge_commit is required.*"), }, { - Config: fmt.Sprintf(config, testRepoName, testAccConf.testRepositoryVisibility, ""), - ExpectError: regexp.MustCompile(`all of[\s\S]*?allow_merge_commit[\s\S]*?must[\s\S]be[\s\S]specified`), + Config: fmt.Sprintf(config, testRepoName, testAccConf.testRepositoryVisibility, ""), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("allow_merge_commit"), knownvalue.Bool(true)), + }, }, }, }, From c8200258d42357aa10928c5b7e955a8b5cc7bc2f Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 29 Mar 2026 23:28:39 +0300 Subject: [PATCH 13/15] Improve structure of `setResourceFieldsFromRepo` Signed-off-by: Timo Sand --- github/resource_github_repository.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index f11dd811b6..db955085af 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -815,7 +815,7 @@ func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, return diag.FromErr(err) } - if diags := setRepositoryFieldsFromRepo(ctx, d, client, owner, finalRepo); diags.HasError() { + if diags := setResourceFieldsFromRepo(ctx, d, client, owner, finalRepo); diags.HasError() { return diags } @@ -858,7 +858,7 @@ func resourceGithubRepositoryRead(ctx context.Context, d *schema.ResourceData, m return diag.FromErr(err) } - if diags := setRepositoryFieldsFromRepo(ctx, d, client, owner, repo); diags.HasError() { + if diags := setResourceFieldsFromRepo(ctx, d, client, owner, repo); diags.HasError() { return diags } @@ -967,7 +967,7 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, repo = updatedRepo } - if diags := setRepositoryFieldsFromRepo(ctx, d, client, owner, repo); diags.HasError() { + if diags := setResourceFieldsFromRepo(ctx, d, client, owner, repo); diags.HasError() { return diags } @@ -1007,11 +1007,10 @@ func resourceGithubRepositoryImport(ctx context.Context, d *schema.ResourceData, return []*schema.ResourceData{d}, nil } -// setRepositoryFieldsFromRepo populates all Computed and Optional+Computed schema fields -// from a *github.Repository API response. Used by Create and Update to set state directly -// instead of chaining to Read. For fields requiring separate API calls (pages, vulnerability -// alerts), targeted calls are made here. -func setRepositoryFieldsFromRepo(ctx context.Context, d *schema.ResourceData, client *github.Client, owner string, repo *github.Repository) diag.Diagnostics { +// setResourceFieldsFromRepo populates all Computed schema fields +// from a *github.Repository API response. +// For fields requiring separate API calls (pages, vulnerability alerts), targeted calls are made here. +func setResourceFieldsFromRepo(ctx context.Context, d *schema.ResourceData, client *github.Client, owner string, repo *github.Repository) diag.Diagnostics { repoName := repo.GetName() if err := errors.Join( @@ -1093,7 +1092,6 @@ func setRepositoryFieldsFromRepo(ctx context.Context, d *schema.ResourceData, cl } } - // Pages (requires separate API call for computed fields like url, status) if repo.GetHasPages() { pages, _, err := client.Repositories.GetPagesInfo(ctx, owner, repoName) if err != nil { @@ -1104,7 +1102,6 @@ func setRepositoryFieldsFromRepo(ctx context.Context, d *schema.ResourceData, cl } } - // Security and analysis + vulnerability alerts (requires separate API call) if repo.GetSecurityAndAnalysis() != nil { vulnerabilityAlerts, _, err := client.Repositories.GetVulnerabilityAlerts(ctx, owner, repoName) if err != nil { From 503e6d152fb0d3059412487f623936adac1ed2d2 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 30 Mar 2026 08:54:34 +0300 Subject: [PATCH 14/15] Cleaning some tests Signed-off-by: Timo Sand --- github/resource_github_repository_test.go | 60 +++++++++++------------ 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/github/resource_github_repository_test.go b/github/resource_github_repository_test.go index c38bc29c9a..7d76fedf4e 100644 --- a/github/resource_github_repository_test.go +++ b/github/resource_github_repository_test.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-testing/compare" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/knownvalue" @@ -64,34 +65,33 @@ func TestAccGithubRepository(t *testing.T) { oldName := fmt.Sprintf(`%srename-%s`, testResourcePrefix, randomID) newName := fmt.Sprintf(`%s-renamed`, oldName) - config := fmt.Sprintf(` + config := ` resource "github_repository" "test" { name = "%[1]s" description = "Terraform acceptance tests %[2]s" visibility = "%s" } - `, oldName, randomID, testAccConf.testRepositoryVisibility) + ` + nameDiffer := statecheck.CompareValue(compare.ValuesDiffer()) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: config, + Config: fmt.Sprintf(config, oldName, randomID, testAccConf.testRepositoryVisibility), ConfigStateChecks: []statecheck.StateCheck{ statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("name"), knownvalue.StringExact(oldName)), statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("full_name"), knownvalue.StringRegexp(regexp.MustCompile(regexp.QuoteMeta(oldName)))), + nameDiffer.AddStateValue("github_repository.test", tfjsonpath.New("name")), }, }, { - // Rename the repo to something else - Config: strings.Replace( - config, - oldName, - newName, 1), + Config: fmt.Sprintf(config, newName, randomID, testAccConf.testRepositoryVisibility), ConfigStateChecks: []statecheck.StateCheck{ statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("name"), knownvalue.StringExact(newName)), statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("full_name"), knownvalue.StringRegexp(regexp.MustCompile(regexp.QuoteMeta(newName)))), + nameDiffer.AddStateValue("github_repository.test", tfjsonpath.New("name")), }, }, }, @@ -199,11 +199,11 @@ resource "github_repository" "test" { t.Run("manages the default branch feature for a repository", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) testRepoName := fmt.Sprintf("%sbranch-%s", testResourcePrefix, randomID) - config := fmt.Sprintf(` + config := ` resource "github_repository" "test" { name = "%s" description = "Terraform acceptance tests %[1]s" - default_branch = "main" + default_branch = "%s" auto_init = true visibility = "%s" } @@ -212,39 +212,36 @@ resource "github_repository" "test" { repository = github_repository.test.name branch = "default" } - `, testRepoName, testAccConf.testRepositoryVisibility) + ` + defaultBranchChangeCheck := statecheck.CompareValue(compare.ValuesDiffer()) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: config, + Config: fmt.Sprintf(config, testRepoName, "main", testAccConf.testRepositoryVisibility), ConfigStateChecks: []statecheck.StateCheck{ - statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("default_branch"), knownvalue.StringExact("main")), + defaultBranchChangeCheck.AddStateValue("github_repository.test", tfjsonpath.New("default_branch")), }, }, - // Test changing default_branch { - Config: strings.Replace(config, - `default_branch = "main"`, - `default_branch = "default"`, 1), + Config: fmt.Sprintf(config, testRepoName, "default", testAccConf.testRepositoryVisibility), ConfigStateChecks: []statecheck.StateCheck{ - statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("default_branch"), knownvalue.StringExact("default")), + defaultBranchChangeCheck.AddStateValue("github_repository.test", tfjsonpath.New("default_branch")), }, }, - // Test changing default_branch back to main again { - Config: config, + Config: fmt.Sprintf(config, testRepoName, "main", testAccConf.testRepositoryVisibility), ConfigStateChecks: []statecheck.StateCheck{ - statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("default_branch"), knownvalue.StringExact("main")), + defaultBranchChangeCheck.AddStateValue("github_repository.test", tfjsonpath.New("default_branch")), }, }, }, }) }) - t.Run("allows setting default_branch on an empty repository", func(t *testing.T) { + t.Run("updates_default_branchon_an_empty_repository_without_error", func(t *testing.T) { // Although default_branch is deprecated, for backwards compatibility // we allow setting it to "main". @@ -259,26 +256,25 @@ resource "github_repository" "test" { } `, testRepoName, testAccConf.testRepositoryVisibility) - defaultBranchChecks := []statecheck.StateCheck{ - statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("default_branch"), knownvalue.StringExact("main")), - } - + defaultBranchChangeCheck := statecheck.CompareValue(compare.ValuesSame()) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ - // Test creation with default_branch set { - Config: config, - ConfigStateChecks: defaultBranchChecks, + Config: config, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("default_branch"), knownvalue.StringExact("main")), + defaultBranchChangeCheck.AddStateValue("github_repository.test", tfjsonpath.New("default_branch")), + }, }, - // Test that changing another property does not try to set - // default_branch (which would crash). { Config: strings.Replace(config, `acceptance tests`, `acceptance test`, 1), - ConfigStateChecks: defaultBranchChecks, + ConfigStateChecks: []statecheck.StateCheck{ + defaultBranchChangeCheck.AddStateValue("github_repository.test", tfjsonpath.New("default_branch")), + }, }, }, }) From 0b52c2c951c3dc1a409962b0d57f799b4189507f Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 30 Mar 2026 09:00:02 +0300 Subject: [PATCH 15/15] Enable primary_language test Signed-off-by: Timo Sand --- github/resource_github_repository_test.go | 68 +++++++++++------------ 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/github/resource_github_repository_test.go b/github/resource_github_repository_test.go index 7d76fedf4e..40bd2aab6d 100644 --- a/github/resource_github_repository_test.go +++ b/github/resource_github_repository_test.go @@ -832,41 +832,39 @@ resource "github_repository" "test" { }) }) - // t.Run("create a repository with go as primary_language", func(t *testing.T) { - // randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) - // testResourceName := fmt.Sprintf("%srepo-%s", testResourcePrefix, randomID) - // config := fmt.Sprintf(` - // resource "github_repository" "test" { - // name = "%s" - // auto_init = true - // } - // resource "github_repository_file" "test" { - // repository = github_repository.test.name - // file = "test.go" - // content = "package main" - // } - // `, testResourceName) - - // check := resource.ComposeTestCheckFunc( - // resource.TestCheckResourceAttr("github_repository.test", "primary_language", "Go"), - // ) - - // resource.ParallelTest(t, resource.TestCase{ - // PreCheck: func() { skipUnauthenticated(t) }, - // ProviderFactories: providerFactories, - // Steps: []resource.TestStep{ - // { - // // Not doing any checks since the file needs to be created before the language can be updated - // Config: config, - // }, - // { - // // Re-running the terraform will refresh the language since the go-file has been created - // Config: config, - // Check: check, - // }, - // }, - // }) - // }) + t.Run("create a repository with go as primary_language", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + testResourceName := fmt.Sprintf("%srepo-%s", testResourcePrefix, randomID) + config := fmt.Sprintf(` + resource "github_repository" "test" { + name = "%s" + auto_init = true + } + resource "github_repository_file" "test" { + repository = github_repository.test.name + file = "test.go" + content = "package main" + } + `, testResourceName) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + // Not doing any checks since the file needs to be created before the language can be updated + Config: config, + }, + { + // Re-running the terraform will refresh the language since the go-file has been created + Config: config, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository.test", tfjsonpath.New("primary_language"), knownvalue.StringExact("Go")), + }, + }, + }, + }) + }) t.Run("manages the legacy pages feature for a repository", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)