From 48fc537778bd21500b2c788a4ea168dfe2a0d1d6 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 14:43:13 +0200 Subject: [PATCH 01/20] Switch from `NewSubsystemLoggingHTTPTransport` to `NewLoggingHTTPTransport` Updated the logging transport in the RateLimitedHTTPClient function to use NewLoggingHTTPTransport. As the subsystem would need to be explicitly inititated in each resource Signed-off-by: Timo Sand --- github/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/config.go b/github/config.go index d5f561c19f..de232c1766 100644 --- a/github/config.go +++ b/github/config.go @@ -59,7 +59,7 @@ var ( func RateLimitedHTTPClient(client *http.Client, writeDelay, readDelay, retryDelay time.Duration, parallelRequests bool, retryableErrors map[int]bool, maxRetries int) *http.Client { client.Transport = NewEtagTransport(client.Transport) client.Transport = NewRateLimitTransport(client.Transport, WithWriteDelay(writeDelay), WithReadDelay(readDelay), WithParallelRequests(parallelRequests)) - client.Transport = logging.NewSubsystemLoggingHTTPTransport("GitHub", client.Transport) + client.Transport = logging.NewLoggingHTTPTransport(client.Transport) client.Transport = newPreviewHeaderInjectorTransport(map[string]string{ // TODO: remove when Stone Crop preview is moved to general availability in the GraphQL API "Accept": "application/vnd.github.stone-crop-preview+json", From f66df44325e2304ed7eff8b24e5431c5e53f7ae8 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 17:43:47 +0200 Subject: [PATCH 02/20] Ensure `update_allows_fetch_and_merge` isn't added for Org Ruleset Signed-off-by: Timo Sand --- github/util_rules.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/github/util_rules.go b/github/util_rules.go index 83a06551a9..38f2a0c18b 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -516,10 +516,14 @@ func flattenRules(rules *github.RepositoryRulesetRules, org bool) []any { // Update rule with parameters if rules.Update != nil { rulesMap["update"] = true - rulesMap["update_allows_fetch_and_merge"] = rules.Update.UpdateAllowsFetchAndMerge + if !org { + rulesMap["update_allows_fetch_and_merge"] = rules.Update.UpdateAllowsFetchAndMerge + } } else { rulesMap["update"] = false - rulesMap["update_allows_fetch_and_merge"] = false + if !org { + rulesMap["update_allows_fetch_and_merge"] = false + } } // Required deployments rule if rules.RequiredDeployments != nil { requiredDeploymentsSlice := make([]map[string]any, 0) From c721d5f579fd9c80c7d13d8d912933ae4dd5d0ec Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 21:58:01 +0200 Subject: [PATCH 03/20] Need to have one of `repository_name` or `repository_id` defined Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 28 +++++++++++++++++-- github/util_rules.go | 12 ++++---- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 9ebd68c4bc..fac9e37718 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -12,8 +12,19 @@ import ( func TestAccGithubOrganizationRuleset(t *testing.T) { t.Run("create_branch_ruleset", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + repoName := fmt.Sprintf("%srepo-org-ruleset-%s", testResourcePrefix, randomID) config := fmt.Sprintf(` +resource "github_repository" "test" { + name = "%s" + visibility = "private" +} + +resource "github_actions_repository_access_level" "test" { + repository = github_repository.test.name + access_level = "organization" +} + resource "github_organization_ruleset" "test" { name = "test-%s" target = "branch" @@ -79,8 +90,8 @@ resource "github_organization_ruleset" "test" { required_workflows { do_not_enforce_on_create = true required_workflow { - path = "path/to/workflow.yaml" - repository_id = 1234 + path = ".github/workflows/echo.yaml" + repository_id = github_repository.test.repo_id } } @@ -102,7 +113,7 @@ resource "github_organization_ruleset" "test" { non_fast_forward = true } } -`, randomID) +`, repoName, randomID) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasPaidOrgs(t) }, @@ -165,6 +176,11 @@ resource "github_organization_ruleset" "test" { include = ["~ALL"] exclude = [] } + + ref_name { + include = ["~ALL"] + exclude = [] + } } rules { @@ -367,6 +383,12 @@ resource "github_organization_ruleset" "test" { } conditions { + + repository_name { + include = ["~ALL"] + exclude = [] + } + ref_name { include = ["~ALL"] exclude = [] diff --git a/github/util_rules.go b/github/util_rules.go index 38f2a0c18b..c0e9aa3d3c 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -516,14 +516,14 @@ func flattenRules(rules *github.RepositoryRulesetRules, org bool) []any { // Update rule with parameters if rules.Update != nil { rulesMap["update"] = true - if !org { - rulesMap["update_allows_fetch_and_merge"] = rules.Update.UpdateAllowsFetchAndMerge - } + if !org { + rulesMap["update_allows_fetch_and_merge"] = rules.Update.UpdateAllowsFetchAndMerge + } } else { rulesMap["update"] = false - if !org { - rulesMap["update_allows_fetch_and_merge"] = false - } + if !org { + rulesMap["update_allows_fetch_and_merge"] = false + } } // Required deployments rule if rules.RequiredDeployments != nil { requiredDeploymentsSlice := make([]map[string]any, 0) From 54bef0714e4e882fcb485ab39f42126bace31591 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 22:00:06 +0200 Subject: [PATCH 04/20] Update indentation Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index fac9e37718..1abc5257b5 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -177,10 +177,10 @@ resource "github_organization_ruleset" "test" { exclude = [] } - ref_name { - include = ["~ALL"] - exclude = [] - } + ref_name { + include = ["~ALL"] + exclude = [] + } } rules { From e2e214c1fe45799d7dee3369546a6c54c78118f7 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 23:09:19 +0200 Subject: [PATCH 05/20] Add handling of `AllowedMergeMethods` It's a required field in the API and go-github doesn't use `omitempty`, so it submits `nil` if it isn't sent explicitly. This change tries to keep it in the state, without having a configuration option for it (poor choice?) And it defaults to all 3 available merge methods if it can't set something from the state. Signed-off-by: Timo Sand --- github/util_rules.go | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/github/util_rules.go b/github/util_rules.go index c0e9aa3d3c..5c04deaa8f 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -1,6 +1,7 @@ package github import ( + "log" "reflect" "sort" @@ -8,6 +9,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) +var DEFAULT_PULL_REQUEST_MERGE_METHODS = []github.PullRequestMergeMethod{github.PullRequestMergeMethodMerge, github.PullRequestMergeMethodRebase, github.PullRequestMergeMethodSquash} + // Helper function to safely convert interface{} to int, handling both int and float64. func toInt(v any) int { switch val := v.(type) { @@ -36,6 +39,24 @@ func toInt64(v any) int64 { } } +func toPullRequestMergeMethods(input []string) []github.PullRequestMergeMethod { + mergeMethods := make([]github.PullRequestMergeMethod, 0) + for _, method := range input { + switch method { + case "merge": + mergeMethods = append(mergeMethods, github.PullRequestMergeMethodMerge) + case "rebase": + mergeMethods = append(mergeMethods, github.PullRequestMergeMethodRebase) + case "squash": + mergeMethods = append(mergeMethods, github.PullRequestMergeMethodSquash) + } + } + if len(mergeMethods) == 0 { + mergeMethods = append(mergeMethods, github.PullRequestMergeMethodMerge) // We need to send at least one method to the API. Defaulting to merge. + } + return mergeMethods +} + func resourceGithubRulesetObject(d *schema.ResourceData, org string) github.RepositoryRuleset { isOrgLevel := len(org) > 0 @@ -295,13 +316,19 @@ func expandRules(input []any, org bool) *github.RepositoryRulesetRules { // Pull request rule if v, ok := rulesMap["pull_request"].([]any); ok && len(v) != 0 { pullRequestMap := v[0].(map[string]any) + allowedMergeMethods := pullRequestMap["allowed_merge_methods"] + if allowedMergeMethods != nil { + allowedMergeMethods = toPullRequestMergeMethods(allowedMergeMethods.([]string)) + } else { + allowedMergeMethods = DEFAULT_PULL_REQUEST_MERGE_METHODS + } params := &github.PullRequestRuleParameters{ - AllowedMergeMethods: []github.PullRequestMergeMethod{github.PullRequestMergeMethodMerge, github.PullRequestMergeMethodSquash, github.PullRequestMergeMethodRebase}, DismissStaleReviewsOnPush: pullRequestMap["dismiss_stale_reviews_on_push"].(bool), RequireCodeOwnerReview: pullRequestMap["require_code_owner_review"].(bool), RequireLastPushApproval: pullRequestMap["require_last_push_approval"].(bool), RequiredApprovingReviewCount: toInt(pullRequestMap["required_approving_review_count"]), RequiredReviewThreadResolution: pullRequestMap["required_review_thread_resolution"].(bool), + AllowedMergeMethods: allowedMergeMethods.([]github.PullRequestMergeMethod), } rulesetRules.PullRequest = params } @@ -542,7 +569,9 @@ func flattenRules(rules *github.RepositoryRulesetRules, org bool) []any { "require_last_push_approval": rules.PullRequest.RequireLastPushApproval, "required_approving_review_count": rules.PullRequest.RequiredApprovingReviewCount, "required_review_thread_resolution": rules.PullRequest.RequiredReviewThreadResolution, + "allowed_merge_methods": rules.PullRequest.AllowedMergeMethods, }) + log.Printf("[DEBUG] Flattened Pull Request rules slice request slice: %#v", pullRequestSlice) rulesMap["pull_request"] = pullRequestSlice } From 9ad68263d001df0532bbae68d022752541b33187 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:19:04 +0200 Subject: [PATCH 06/20] Add `allowed_merge_methods` to `rules` for Org & Repo rulesets This is a required field and the SDK doesn't omit if it's empty Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 10 ++++++++++ github/resource_github_repository_ruleset.go | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 46cca07781..c93bf026da 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -198,6 +198,16 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "Require all commits be made to a non-target branch and submitted via a pull request before they can be merged.", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ + "allowed_merge_methods": { + Type: schema.TypeList, + Required: true, + MinItems: 1, + Description: "Array of allowed merge methods. Allowed values include `merge`, `squash`, and `rebase`. At least one option must be enabled.", + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateDiagFunc: toDiagFunc(validation.StringInSlice([]string{"merge", "squash", "rebase"}, false), "allowed_merge_methods"), + }, + }, "dismiss_stale_reviews_on_push": { Type: schema.TypeBool, Optional: true, diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index 67ca60aa6a..ebb5400a75 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -189,6 +189,16 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Description: "Require all commits be made to a non-target branch and submitted via a pull request before they can be merged.", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ + "allowed_merge_methods": { + Type: schema.TypeList, + Required: true, + MinItems: 1, + Description: "Array of allowed merge methods. Allowed values include `merge`, `squash`, and `rebase`. At least one option must be enabled.", + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateDiagFunc: toDiagFunc(validation.StringInSlice([]string{"merge", "squash", "rebase"}, false), "allowed_merge_methods"), + }, + }, "dismiss_stale_reviews_on_push": { Type: schema.TypeBool, Optional: true, From be4009388645777bffb5d488cd3661e672118b4a Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:20:18 +0200 Subject: [PATCH 07/20] Fixed type conversion for `allowed_merge_methods` Signed-off-by: Timo Sand --- github/util_rules.go | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/github/util_rules.go b/github/util_rules.go index 5c04deaa8f..83c01e50f5 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -39,20 +39,17 @@ func toInt64(v any) int64 { } } -func toPullRequestMergeMethods(input []string) []github.PullRequestMergeMethod { - mergeMethods := make([]github.PullRequestMergeMethod, 0) - for _, method := range input { - switch method { - case "merge": - mergeMethods = append(mergeMethods, github.PullRequestMergeMethodMerge) - case "rebase": - mergeMethods = append(mergeMethods, github.PullRequestMergeMethodRebase) - case "squash": - mergeMethods = append(mergeMethods, github.PullRequestMergeMethodSquash) - } +func toPullRequestMergeMethods(input any) []github.PullRequestMergeMethod { + value, ok := input.([]any) + if !ok || value == nil || len(value) == 0 { + log.Printf("[DEBUG] No allowed merge methods provided, using default: %#v", input) + return DEFAULT_PULL_REQUEST_MERGE_METHODS } - if len(mergeMethods) == 0 { - mergeMethods = append(mergeMethods, github.PullRequestMergeMethodMerge) // We need to send at least one method to the API. Defaulting to merge. + mergeMethods := make([]github.PullRequestMergeMethod, 0, len(value)) + for _, item := range value { + if method, ok := item.(string); ok { + mergeMethods = append(mergeMethods, github.PullRequestMergeMethod(method)) + } } return mergeMethods } @@ -317,18 +314,14 @@ func expandRules(input []any, org bool) *github.RepositoryRulesetRules { if v, ok := rulesMap["pull_request"].([]any); ok && len(v) != 0 { pullRequestMap := v[0].(map[string]any) allowedMergeMethods := pullRequestMap["allowed_merge_methods"] - if allowedMergeMethods != nil { - allowedMergeMethods = toPullRequestMergeMethods(allowedMergeMethods.([]string)) - } else { - allowedMergeMethods = DEFAULT_PULL_REQUEST_MERGE_METHODS - } + params := &github.PullRequestRuleParameters{ DismissStaleReviewsOnPush: pullRequestMap["dismiss_stale_reviews_on_push"].(bool), RequireCodeOwnerReview: pullRequestMap["require_code_owner_review"].(bool), RequireLastPushApproval: pullRequestMap["require_last_push_approval"].(bool), RequiredApprovingReviewCount: toInt(pullRequestMap["required_approving_review_count"]), RequiredReviewThreadResolution: pullRequestMap["required_review_thread_resolution"].(bool), - AllowedMergeMethods: allowedMergeMethods.([]github.PullRequestMergeMethod), + AllowedMergeMethods: toPullRequestMergeMethods(allowedMergeMethods), } rulesetRules.PullRequest = params } From 5ab94ed21fe5dcd6a1d76d58fdd5f60c49df0945 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 5 Dec 2025 00:24:47 +0200 Subject: [PATCH 08/20] Update test content to actually pass the GH API Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 1abc5257b5..3b992eaceb 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -18,6 +18,17 @@ func TestAccGithubOrganizationRuleset(t *testing.T) { resource "github_repository" "test" { name = "%s" visibility = "private" + auto_init = true +} + +resource "github_repository_file" "workflow_file" { + repository = github_repository.test.name + branch = "main" + file = ".github/workflows/echo.yaml" + content = "name: Echo Workflow\n\non: [pull_request]\n\njobs:\n echo:\n runs-on: linux\n steps:\n - run: echo \"Hello, world!\"\n" + commit_message = "Managed by Terraform" + commit_author = "Terraform User" + commit_email = "terraform@example.com" } resource "github_actions_repository_access_level" "test" { From d4bd616f48b413089c96ed1df826fa83874985da Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:17:34 +0200 Subject: [PATCH 09/20] Enable `TestGithubOrganizationRulesets/Creates_and_updates_organization_rulesets_without_errors` to pass Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 3b992eaceb..f5d6758514 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -13,18 +13,22 @@ func TestAccGithubOrganizationRuleset(t *testing.T) { t.Run("create_branch_ruleset", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) repoName := fmt.Sprintf("%srepo-org-ruleset-%s", testResourcePrefix, randomID) + rulesetName := fmt.Sprintf("%s-branch-ruleset-%s", testResourcePrefix, randomID) + + workflowFilePath := ".github/workflows/echo.yaml" config := fmt.Sprintf(` resource "github_repository" "test" { name = "%s" visibility = "private" auto_init = true + ignore_vulnerability_alerts_during_read = true } resource "github_repository_file" "workflow_file" { repository = github_repository.test.name branch = "main" - file = ".github/workflows/echo.yaml" + file = "%[3]s" content = "name: Echo Workflow\n\non: [pull_request]\n\njobs:\n echo:\n runs-on: linux\n steps:\n - run: echo \"Hello, world!\"\n" commit_message = "Managed by Terraform" commit_author = "Terraform User" @@ -37,7 +41,7 @@ resource "github_actions_repository_access_level" "test" { } resource "github_organization_ruleset" "test" { - name = "test-%s" + name = "%[2]s" target = "branch" enforcement = "active" @@ -81,6 +85,7 @@ resource "github_organization_ruleset" "test" { required_signatures = false pull_request { + allowed_merge_methods = ["merge", "rebase", "squash"] required_approving_review_count = 2 required_review_thread_resolution = true require_code_owner_review = true @@ -101,8 +106,9 @@ resource "github_organization_ruleset" "test" { required_workflows { do_not_enforce_on_create = true required_workflow { - path = ".github/workflows/echo.yaml" + path = "%[3]s" repository_id = github_repository.test.repo_id + ref = "main" # Default ref is master } } @@ -123,8 +129,9 @@ resource "github_organization_ruleset" "test" { non_fast_forward = true } + depends_on = [github_repository_file.workflow_file] } -`, repoName, randomID) +`, repoName, rulesetName, workflowFilePath) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasPaidOrgs(t) }, @@ -133,7 +140,7 @@ resource "github_organization_ruleset" "test" { { Config: config, Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_organization_ruleset.test", "name", "test"), + resource.TestCheckResourceAttr("github_organization_ruleset.test", "name", rulesetName), resource.TestCheckResourceAttr("github_organization_ruleset.test", "target", "branch"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "enforcement", "active"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "bypass_actors.#", "3"), @@ -145,8 +152,8 @@ resource "github_organization_ruleset" "test" { resource.TestCheckResourceAttr("github_organization_ruleset.test", "bypass_actors.2.actor_id", "1"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "bypass_actors.2.actor_type", "OrganizationAdmin"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "bypass_actors.2.bypass_mode", "always"), - resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.required_workflows.0.o_not_enforce_on_create", "true"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.required_workflows.0.required_workflow.0.path", "path/to/workflow.yaml"), - resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.required_workflows.0.required_workflow.0.repository_id", "1234"), + resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.required_workflows.0.do_not_enforce_on_create", "true"), + resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.required_workflows.0.required_workflow.0.path", workflowFilePath), resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.required_code_scanning.0.required_code_scanning_tool.0.alerts_threshold", "errors"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.required_code_scanning.0.required_code_scanning_tool.0.security_alerts_threshold", "high_or_higher"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.required_code_scanning.0.required_code_scanning_tool.0.tool", "CodeQL"), @@ -158,10 +165,11 @@ resource "github_organization_ruleset" "test" { t.Run("create_push_ruleset", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + rulesetName := fmt.Sprintf("%s-push-ruleset-%s", testResourcePrefix, randomID) config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { - name = "test-%s" + name = "%s" target = "push" enforcement = "active" @@ -200,7 +208,7 @@ resource "github_organization_ruleset" "test" { } max_file_size { - max_file_size = 1048576 + max_file_size = 99 } file_extension_restriction { @@ -208,7 +216,7 @@ resource "github_organization_ruleset" "test" { } } } -`, randomID) +`, rulesetName) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasPaidOrgs(t) }, @@ -217,7 +225,7 @@ resource "github_organization_ruleset" "test" { { Config: config, Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("github_organization_ruleset.test", "name", "test"), + resource.TestCheckResourceAttr("github_organization_ruleset.test", "name", rulesetName), resource.TestCheckResourceAttr("github_organization_ruleset.test", "target", "push"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "enforcement", "active"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "bypass_actors.#", "3"), @@ -230,7 +238,7 @@ resource "github_organization_ruleset" "test" { resource.TestCheckResourceAttr("github_organization_ruleset.test", "bypass_actors.2.actor_type", "OrganizationAdmin"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "bypass_actors.2.bypass_mode", "always"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.file_path_restriction.0.restricted_file_paths.0", "test.txt"), - resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.max_file_size.0.max_file_size", "1048576"), + resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.max_file_size.0.max_file_size", "99"), resource.TestCheckResourceAttr("github_organization_ruleset.test", "rules.0.file_extension_restriction.0.restricted_file_extensions.0", "*.zip"), ), }, @@ -289,10 +297,11 @@ resource "github_organization_ruleset" "test" { t.Run("update_clear_bypass_actors", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + rulesetName := fmt.Sprintf("%s-bypass-ruleset-%s", testResourcePrefix, randomID) config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { - name = "test-%s" + name = "%s" target = "branch" enforcement = "active" @@ -329,11 +338,11 @@ resource "github_organization_ruleset" "test" { creation = true } } -`, randomID) +`, rulesetName) - configUpdated := ` + configUpdated := fmt.Sprintf(` resource "github_organization_ruleset" "test" { - name = "test-bypass" + name = "%s" target = "branch" enforcement = "active" @@ -353,7 +362,7 @@ resource "github_organization_ruleset" "test" { creation = true } } -` +`, rulesetName) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasPaidOrgs(t) }, From 9b2c813bd27440570688033d0f6f70c9ea5362d3 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:59:04 +0200 Subject: [PATCH 10/20] Add workaround for GH API bug that OrgAdmin actor_id is returned as `null` Signed-off-by: Timo Sand --- github/util_rules.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/github/util_rules.go b/github/util_rules.go index 83c01e50f5..a0ddf25b3f 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -133,9 +133,15 @@ func flattenBypassActors(bypassActors []*github.BypassActor) []any { actorsSlice := make([]any, 0) for _, v := range bypassActors { actorMap := make(map[string]any) - - actorMap["actor_id"] = v.GetActorID() - actorMap["actor_type"] = v.GetActorType() + actorID := v.GetActorID() + actorType := v.GetActorType() + if *actorType == github.BypassActorTypeOrganizationAdmin && actorID == 0 { + // This is a workaround for the GitHub API bug where OrganizationAdmin actor_id is returned as `null` instead of `1` + log.Printf("[DEBUG] Setting OrganizationAdmin Actor ID to 1") + actorID = 1 + } + actorMap["actor_id"] = actorID + actorMap["actor_type"] = actorType actorMap["bypass_mode"] = v.GetBypassMode() actorsSlice = append(actorsSlice, actorMap) From 5a79ef4491fb3881566d34b2d844fb378e560a6c Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 20:35:11 +0200 Subject: [PATCH 11/20] `make fmt` Signed-off-by: Timo Sand --- github/util_rules.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/util_rules.go b/github/util_rules.go index a0ddf25b3f..bf68764972 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -136,8 +136,8 @@ func flattenBypassActors(bypassActors []*github.BypassActor) []any { actorID := v.GetActorID() actorType := v.GetActorType() if *actorType == github.BypassActorTypeOrganizationAdmin && actorID == 0 { - // This is a workaround for the GitHub API bug where OrganizationAdmin actor_id is returned as `null` instead of `1` - log.Printf("[DEBUG] Setting OrganizationAdmin Actor ID to 1") + // This is a workaround for the GitHub API bug where OrganizationAdmin actor_id is returned as `null` instead of `1` + log.Printf("[DEBUG] Setting OrganizationAdmin Actor ID to 1") actorID = 1 } actorMap["actor_id"] = actorID From e106555dcd9eac1fdc4ae2aa39f620fd0d957f5f Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 02:01:15 +0200 Subject: [PATCH 12/20] Fix `TestGithubOrganizationRulesets/Creates_organization_ruleset_with_all_bypass_modes` Main fix: `bypass_actors` is returned as sorted from GH API so tests need re-indexing Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index c93bf026da..86cf8d034d 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -46,7 +46,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "Possible values for Enforcement are `disabled`, `active`, `evaluate`. Note: `evaluate` is currently only supported for owners of type `organization`.", }, "bypass_actors": { - Type: schema.TypeList, + Type: schema.TypeList, // TODO: These are returned from GH API sorted by actor_id, we might want to investigate if we want to include sorting Optional: true, DiffSuppressFunc: bypassActorsDiffSuppressFunc, Description: "The actors that can bypass the rules in this ruleset.", From 358c381c1fded187e6c919f59b26eca1a0c7455a Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 9 Jan 2026 01:35:59 +0200 Subject: [PATCH 13/20] Update github/resource_github_organization_ruleset_test.go Co-authored-by: Steve Hipwell --- github/resource_github_organization_ruleset_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index f5d6758514..d7468b310d 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -29,7 +29,17 @@ resource "github_repository_file" "workflow_file" { repository = github_repository.test.name branch = "main" file = "%[3]s" - content = "name: Echo Workflow\n\non: [pull_request]\n\njobs:\n echo:\n runs-on: linux\n steps:\n - run: echo \"Hello, world!\"\n" + content = < Date: Fri, 9 Jan 2026 01:36:05 +0200 Subject: [PATCH 14/20] Update github/resource_github_organization_ruleset_test.go Co-authored-by: Steve Hipwell --- github/resource_github_organization_ruleset_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index d7468b310d..9a4baafc66 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -116,7 +116,7 @@ resource "github_organization_ruleset" "test" { required_workflows { do_not_enforce_on_create = true required_workflow { - path = "%[3]s" + path = github_repository_file.workflow_file.path repository_id = github_repository.test.repo_id ref = "main" # Default ref is master } From fac074eaf539a33bf73dd2e83d496d9ddaa19c1a Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 9 Jan 2026 01:36:12 +0200 Subject: [PATCH 15/20] Update github/resource_github_organization_ruleset_test.go Co-authored-by: Steve Hipwell --- github/resource_github_organization_ruleset_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 9a4baafc66..b4728659e8 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -139,7 +139,6 @@ resource "github_organization_ruleset" "test" { non_fast_forward = true } - depends_on = [github_repository_file.workflow_file] } `, repoName, rulesetName, workflowFilePath) From eaab7240b22b47cc2937c89246fa050b1cacf0ed Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 9 Jan 2026 10:45:51 +0200 Subject: [PATCH 16/20] Fix casing of constant Signed-off-by: Timo Sand --- github/util_rules.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/github/util_rules.go b/github/util_rules.go index bf68764972..37e51ccc18 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -9,7 +9,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) -var DEFAULT_PULL_REQUEST_MERGE_METHODS = []github.PullRequestMergeMethod{github.PullRequestMergeMethodMerge, github.PullRequestMergeMethodRebase, github.PullRequestMergeMethodSquash} +// This is a workaround for the SDK not setting the default value for the allowed_merge_methods field. +var defaultPullRequestMergeMethods = []github.PullRequestMergeMethod{github.PullRequestMergeMethodMerge, github.PullRequestMergeMethodRebase, github.PullRequestMergeMethodSquash} // Helper function to safely convert interface{} to int, handling both int and float64. func toInt(v any) int { @@ -43,7 +44,7 @@ func toPullRequestMergeMethods(input any) []github.PullRequestMergeMethod { value, ok := input.([]any) if !ok || value == nil || len(value) == 0 { log.Printf("[DEBUG] No allowed merge methods provided, using default: %#v", input) - return DEFAULT_PULL_REQUEST_MERGE_METHODS + return defaultPullRequestMergeMethods } mergeMethods := make([]github.PullRequestMergeMethod, 0, len(value)) for _, item := range value { From f120242b21bbfcdf28a651e7be7a10de63974fd6 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 9 Jan 2026 21:21:22 +0200 Subject: [PATCH 17/20] Fix attribute reference Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index b4728659e8..14087d1dfa 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -116,7 +116,7 @@ resource "github_organization_ruleset" "test" { required_workflows { do_not_enforce_on_create = true required_workflow { - path = github_repository_file.workflow_file.path + path = github_repository_file.workflow_file.file repository_id = github_repository.test.repo_id ref = "main" # Default ref is master } From efc7a5891d3dab997842cd3894d2e676f3f61641 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 9 Jan 2026 21:23:30 +0200 Subject: [PATCH 18/20] Refactor `Providers` => `ProviderFactories` in org ruleset tests Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 14087d1dfa..774315e6db 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -374,8 +374,8 @@ resource "github_organization_ruleset" "test" { `, rulesetName) resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessHasPaidOrgs(t) }, - Providers: testAccProviders, + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, @@ -431,8 +431,8 @@ resource "github_organization_ruleset" "test" { ` resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessHasPaidOrgs(t) }, - Providers: testAccProviders, + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: fmt.Sprintf(config, randomID, bypassMode), From 903cd046c4cbc913b4afdb1a453a8229bced64a6 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 9 Jan 2026 21:29:47 +0200 Subject: [PATCH 19/20] Remove "normalizing" `actor_id`` With the recent changes to the SDK and the Create method we less often purely refresh state if it hasn't changed upstream, leading to us ignoring the return value of `actor_id` often Signed-off-by: Timo Sand --- github/util_rules.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/github/util_rules.go b/github/util_rules.go index 37e51ccc18..734801598f 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -136,11 +136,6 @@ func flattenBypassActors(bypassActors []*github.BypassActor) []any { actorMap := make(map[string]any) actorID := v.GetActorID() actorType := v.GetActorType() - if *actorType == github.BypassActorTypeOrganizationAdmin && actorID == 0 { - // This is a workaround for the GitHub API bug where OrganizationAdmin actor_id is returned as `null` instead of `1` - log.Printf("[DEBUG] Setting OrganizationAdmin Actor ID to 1") - actorID = 1 - } actorMap["actor_id"] = actorID actorMap["actor_type"] = actorType actorMap["bypass_mode"] = v.GetBypassMode() From ac1b510d1f762017fb783013f6be54dbda1b8614 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 9 Jan 2026 22:11:03 +0200 Subject: [PATCH 20/20] Refactor to use `tflog` in `resource_github_organization_ruleset.go` Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 131 ++++++++++++++++-- 1 file changed, 119 insertions(+), 12 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 86cf8d034d..eea93ffc0a 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -4,11 +4,11 @@ import ( "context" "errors" "fmt" - "log" "net/http" "strconv" "github.com/google/go-github/v81/github" + "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -598,13 +598,23 @@ func resourceGithubOrganizationRuleset() *schema.Resource { func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client - owner := meta.(*Owner).name + name := d.Get("name").(string) + + tflog.Debug(ctx, fmt.Sprintf("Creating organization ruleset: %s/%s", owner, name), map[string]any{ + "owner": owner, + "name": name, + }) rulesetReq := resourceGithubRulesetObject(d, owner) ruleset, resp, err := client.Organizations.CreateRepositoryRuleset(ctx, owner, rulesetReq) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Failed to create organization ruleset: %s/%s", owner, name), map[string]any{ + "owner": owner, + "name": name, + "error": err.Error(), + }) return diag.FromErr(err) } @@ -613,16 +623,31 @@ func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.Reso _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("etag", resp.Header.Get("ETag")) + tflog.Info(ctx, fmt.Sprintf("Created organization ruleset: %s/%s (ID: %d)", owner, name, *ruleset.ID), map[string]any{ + "owner": owner, + "name": name, + "ruleset_id": *ruleset.ID, + }) + return nil } func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client - owner := meta.(*Owner).name + tflog.Trace(ctx, fmt.Sprintf("Reading organization ruleset: %s", d.Id()), map[string]any{ + "owner": owner, + "ruleset_id": d.Id(), + }) + rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Could not convert ruleset ID '%s' to int64", d.Id()), map[string]any{ + "owner": owner, + "ruleset_id": d.Id(), + "error": err.Error(), + }) return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } @@ -635,15 +660,26 @@ func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.Resour var ghErr *github.ErrorResponse if errors.As(err, &ghErr) { if ghErr.Response.StatusCode == http.StatusNotModified { + tflog.Debug(ctx, "API responded with StatusNotModified, not refreshing state", map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) return nil } if ghErr.Response.StatusCode == http.StatusNotFound { - log.Printf("[INFO] Removing ruleset %s: %d from state because it no longer exists in GitHub", - owner, rulesetID) + tflog.Info(ctx, fmt.Sprintf("Removing ruleset %s/%d from state because it no longer exists in GitHub", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) d.SetId("") return nil } } + tflog.Error(ctx, fmt.Sprintf("Failed to read organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "error": err.Error(), + }) return diag.FromErr(err) } @@ -657,23 +693,45 @@ func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.Resour _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("etag", resp.Header.Get("ETag")) + tflog.Trace(ctx, fmt.Sprintf("Successfully read organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "name": ruleset.Name, + }) + return nil } func resourceGithubOrganizationRulesetUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client - owner := meta.(*Owner).name - - rulesetReq := resourceGithubRulesetObject(d, owner) + name := d.Get("name").(string) rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Could not convert ruleset ID '%s' to int64", d.Id()), map[string]any{ + "owner": owner, + "ruleset_id": d.Id(), + "error": err.Error(), + }) return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } + tflog.Debug(ctx, fmt.Sprintf("Updating organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "name": name, + }) + + rulesetReq := resourceGithubRulesetObject(d, owner) + ruleset, resp, err := client.Organizations.UpdateRepositoryRuleset(ctx, owner, rulesetID, rulesetReq) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Failed to update organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "error": err.Error(), + }) return diag.FromErr(err) } @@ -682,6 +740,12 @@ func resourceGithubOrganizationRulesetUpdate(ctx context.Context, d *schema.Reso _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("etag", resp.Header.Get("ETag")) + tflog.Info(ctx, fmt.Sprintf("Updated organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "name": name, + }) + return nil } @@ -691,36 +755,79 @@ func resourceGithubOrganizationRulesetDelete(ctx context.Context, d *schema.Reso rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Could not convert ruleset ID '%s' to int64", d.Id()), map[string]any{ + "owner": owner, + "ruleset_id": d.Id(), + "error": err.Error(), + }) return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } - log.Printf("[DEBUG] Deleting organization ruleset: %s: %d", owner, rulesetID) + tflog.Debug(ctx, fmt.Sprintf("Deleting organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) + _, err = client.Organizations.DeleteRepositoryRuleset(ctx, owner, rulesetID) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Failed to delete organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "error": err.Error(), + }) return diag.FromErr(err) } + tflog.Info(ctx, fmt.Sprintf("Deleted organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) + return nil } func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { + client := meta.(*Owner).v3client + owner := meta.(*Owner).name + rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Could not convert ruleset ID '%s' to int64", d.Id()), map[string]any{ + "owner": owner, + "ruleset_id": d.Id(), + "error": err.Error(), + }) return []*schema.ResourceData{d}, unconvertibleIdErr(d.Id(), err) } if rulesetID == 0 { + tflog.Error(ctx, "ruleset_id must be present and non-zero", map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) return []*schema.ResourceData{d}, fmt.Errorf("`ruleset_id` must be present") } - log.Printf("[DEBUG] Importing organization ruleset with ID: %d", rulesetID) - client := meta.(*Owner).v3client - owner := meta.(*Owner).name + tflog.Debug(ctx, fmt.Sprintf("Importing organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) ruleset, _, err := client.Organizations.GetRepositoryRuleset(ctx, owner, rulesetID) if ruleset == nil || err != nil { + tflog.Error(ctx, fmt.Sprintf("Failed to import organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "error": err.Error(), + }) return []*schema.ResourceData{d}, err } d.SetId(strconv.FormatInt(ruleset.GetID(), 10)) + tflog.Info(ctx, fmt.Sprintf("Imported organization ruleset: %s/%d (name: %s)", owner, rulesetID, ruleset.Name), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "name": ruleset.Name, + }) + return []*schema.ResourceData{d}, nil }