From 9909eccc8e3593d84cf14a8c9a5ba64544954e69 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 2 Dec 2025 20:55:40 +0200 Subject: [PATCH 01/72] Ensure tests use `GITHUB_TEST_*` values for Owner and Organization Signed-off-by: Timo Sand --- github/provider_utils.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/github/provider_utils.go b/github/provider_utils.go index 386a4e4407..65a3db48b4 100644 --- a/github/provider_utils.go +++ b/github/provider_utils.go @@ -120,6 +120,7 @@ func testOrganizationFunc() string { organization := os.Getenv("GITHUB_ORGANIZATION") if organization == "" { organization = os.Getenv("GITHUB_TEST_ORGANIZATION") + os.Setenv("GITHUB_ORGANIZATION", organization) } return organization } @@ -128,6 +129,7 @@ func testOwnerFunc() string { owner := os.Getenv("GITHUB_OWNER") if owner == "" { owner = os.Getenv("GITHUB_TEST_OWNER") + os.Setenv("GITHUB_OWNER", owner) } return owner } From d3bafe6eb6577ac5811f593c3b22cf4fed5387b5 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 2 Dec 2025 21:43:10 +0200 Subject: [PATCH 02/72] Update `skipUnlessMode` to check for enterprise ENV variables Signed-off-by: Timo Sand --- github/provider_utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/provider_utils.go b/github/provider_utils.go index 65a3db48b4..7df6a99746 100644 --- a/github/provider_utils.go +++ b/github/provider_utils.go @@ -54,8 +54,8 @@ func skipUnlessMode(t *testing.T, providerMode string) { t.Log("GITHUB_TOKEN environment variable should be empty") } case enterprise: - if os.Getenv("GITHUB_TOKEN") == "" { - t.Log("GITHUB_TOKEN environment variable should be set") + if os.Getenv("GITHUB_TOKEN") == "" || os.Getenv("ENTERPRISE_ACCOUNT") != "true" || os.Getenv("ENTERPRISE_SLUG") == "" { + t.Log("GITHUB_TOKEN and ENTERPRISE_ACCOUNT and ENTERPRISE_SLUG environment variables should be set") } else { return } From a4f929b4441c56dd3ed2ef73cf07c1c3daa894a1 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 12:33:11 +0200 Subject: [PATCH 03/72] Use `testOwnerFunc` to set `testOwner` so that `GITHUB_TEST_OWNER` works properly Signed-off-by: Timo Sand --- github/provider_utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/provider_utils.go b/github/provider_utils.go index 7df6a99746..b7b35561b9 100644 --- a/github/provider_utils.go +++ b/github/provider_utils.go @@ -13,7 +13,7 @@ var ( isPaidPlan = os.Getenv("GITHUB_PAID_FEATURES") testEnterprise = os.Getenv("ENTERPRISE_SLUG") testOrganization = testOrganizationFunc() - testOwner = os.Getenv("GITHUB_OWNER") + testOwner = testOwnerFunc() testToken = os.Getenv("GITHUB_TOKEN") testBaseURLGHES = os.Getenv("GHES_BASE_URL") ) From fbc8707e245edbbd3f7c26e6fb29a62320ce3e16 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 14:43:13 +0200 Subject: [PATCH 04/72] 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 3e0e2643e4..548897c635 100644 --- a/github/config.go +++ b/github/config.go @@ -47,7 +47,7 @@ var GHECDataResidencyHostMatch = regexp.MustCompile(`^[a-zA-Z0-9.\-]+\.ghe\.com\ 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 6966690b274b949075c4977cbefc2a9b360fb475 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 16:57:16 +0200 Subject: [PATCH 05/72] Update to use Context aware CRUD functions By using the `*Context` CRUD functions we get context pass-through and enable HTTP Req/Resp Debug logging Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 47 +++++++++---------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index c0a027dc67..33efdceb3d 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -9,18 +9,19 @@ import ( "strconv" "github.com/google/go-github/v77/github" + "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" ) func resourceGithubOrganizationRuleset() *schema.Resource { return &schema.Resource{ - Create: resourceGithubOrganizationRulesetCreate, - Read: resourceGithubOrganizationRulesetRead, - Update: resourceGithubOrganizationRulesetUpdate, - Delete: resourceGithubOrganizationRulesetDelete, + CreateContext: resourceGithubOrganizationRulesetCreate, + ReadContext: resourceGithubOrganizationRulesetRead, + UpdateContext: resourceGithubOrganizationRulesetUpdate, + DeleteContext: resourceGithubOrganizationRulesetDelete, Importer: &schema.ResourceImporter{ - State: resourceGithubOrganizationRulesetImport, + StateContext: resourceGithubOrganizationRulesetImport, }, SchemaVersion: 1, @@ -585,36 +586,35 @@ func resourceGithubOrganizationRuleset() *schema.Resource { } } -func resourceGithubOrganizationRulesetCreate(d *schema.ResourceData, meta any) error { +func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client rulesetReq := resourceGithubRulesetObject(d, meta.(*Owner).name) org := meta.(*Owner).name - ctx := context.Background() var ruleset *github.RepositoryRuleset var err error ruleset, _, err = client.Organizations.CreateRepositoryRuleset(ctx, org, *rulesetReq) if err != nil { - return err + return diag.FromErr(err) } d.SetId(strconv.FormatInt(*ruleset.ID, 10)) - return resourceGithubOrganizationRulesetRead(d, meta) + return resourceGithubOrganizationRulesetRead(ctx, d, meta) } -func resourceGithubOrganizationRulesetRead(d *schema.ResourceData, meta any) error { +func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client org := meta.(*Owner).name rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { - return unconvertibleIdErr(d.Id(), err) + return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } - ctx := context.WithValue(context.Background(), ctxId, rulesetID) + ctx = context.WithValue(ctx, ctxId, rulesetID) if !d.IsNewResource() { ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string)) } @@ -636,7 +636,7 @@ func resourceGithubOrganizationRulesetRead(d *schema.ResourceData, meta any) err return nil } } - return err + return diag.FromErr(err) } if ruleset == nil { @@ -659,7 +659,7 @@ func resourceGithubOrganizationRulesetRead(d *schema.ResourceData, meta any) err return nil } -func resourceGithubOrganizationRulesetUpdate(d *schema.ResourceData, meta any) error { +func resourceGithubOrganizationRulesetUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client rulesetReq := resourceGithubRulesetObject(d, meta.(*Owner).name) @@ -667,38 +667,38 @@ func resourceGithubOrganizationRulesetUpdate(d *schema.ResourceData, meta any) e org := meta.(*Owner).name rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { - return unconvertibleIdErr(d.Id(), err) + return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } - ctx := context.WithValue(context.Background(), ctxId, rulesetID) + ctx = context.WithValue(ctx, ctxId, rulesetID) var ruleset *github.RepositoryRuleset ruleset, _, err = client.Organizations.UpdateRepositoryRuleset(ctx, org, rulesetID, *rulesetReq) if err != nil { - return err + return diag.FromErr(err) } d.SetId(strconv.FormatInt(*ruleset.ID, 10)) - return resourceGithubOrganizationRulesetRead(d, meta) + return resourceGithubOrganizationRulesetRead(ctx, d, meta) } -func resourceGithubOrganizationRulesetDelete(d *schema.ResourceData, meta any) error { +func resourceGithubOrganizationRulesetDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client org := meta.(*Owner).name rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { - return unconvertibleIdErr(d.Id(), err) + return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } - ctx := context.WithValue(context.Background(), ctxId, rulesetID) + ctx = context.WithValue(ctx, ctxId, rulesetID) log.Printf("[DEBUG] Deleting organization ruleset: %s: %d", org, rulesetID) _, err = client.Organizations.DeleteRepositoryRuleset(ctx, org, rulesetID) - return err + return diag.FromErr(err) } -func resourceGithubOrganizationRulesetImport(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { +func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { return []*schema.ResourceData{d}, unconvertibleIdErr(d.Id(), err) @@ -710,7 +710,6 @@ func resourceGithubOrganizationRulesetImport(d *schema.ResourceData, meta any) ( client := meta.(*Owner).v3client org := meta.(*Owner).name - ctx := context.Background() ruleset, _, err := client.Organizations.GetRepositoryRuleset(ctx, org, rulesetID) if ruleset == nil || err != nil { From a60ac7f576fed3c9afa17da244daa9e2d69faa7b Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 17:43:47 +0200 Subject: [PATCH 06/72] Ensure `update_allows_fetch_and_merge` isn't added for Org Ruleset Signed-off-by: Timo Sand --- github/respository_rules_utils.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/github/respository_rules_utils.go b/github/respository_rules_utils.go index 4044293f52..f2c5fdf44d 100644 --- a/github/respository_rules_utils.go +++ b/github/respository_rules_utils.go @@ -515,10 +515,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 3e2cd38fd9017f77d022c93206a391d9986d481d Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 21:57:26 +0200 Subject: [PATCH 07/72] Update name of resource, to make debugging failing tests easier Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 68e432e688..4b166772e5 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -22,9 +22,10 @@ func TestGithubOrganizationRulesets(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) t.Run("Creates and updates organization rulesets without errors", func(t *testing.T) { + resourceName := "test-create-and-update" config := fmt.Sprintf(` - resource "github_organization_ruleset" "test" { - name = "test-%s" + resource "github_organization_ruleset" "%[1]s" { + name = "test-%[2]s" target = "branch" enforcement = "active" @@ -89,46 +90,46 @@ func TestGithubOrganizationRulesets(t *testing.T) { non_fast_forward = true } } - `, randomID) + `, resourceName, randomID) check := resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( - "github_organization_ruleset.test", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "name", "test", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "enforcement", "active", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "rules.0.required_workflows.0.do_not_enforce_on_create", "true", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "rules.0.required_workflows.0.required_workflow.0.path", "path/to/workflow.yaml", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "rules.0.required_workflows.0.required_workflow.0.repository_id", "1234", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "rules.0.required_code_scanning.0.required_code_scanning_tool.0.alerts_threshold", "errors", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "rules.0.required_code_scanning.0.required_code_scanning_tool.0.security_alerts_threshold", "high_or_higher", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "rules.0.required_code_scanning.0.required_code_scanning_tool.0.tool", "CodeQL", ), From d82c614f542ff00bd2ad756e3cede14c27e7918f Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 21:58:01 +0200 Subject: [PATCH 08/72] Need to have one of `repository_name` or `repository_id` defined Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 4b166772e5..ece0f63485 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -34,6 +34,10 @@ func TestGithubOrganizationRulesets(t *testing.T) { include = ["~ALL"] exclude = [] } + repository_name { + include = ["~ALL"] + exclude = [] + } } rules { From b2c357041facfba82c66a7996ecccd53f66237dc Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 21:58:28 +0200 Subject: [PATCH 09/72] Need to have a valid repo reference Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index ece0f63485..3d93117758 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -24,6 +24,11 @@ func TestGithubOrganizationRulesets(t *testing.T) { t.Run("Creates and updates organization rulesets without errors", func(t *testing.T) { resourceName := "test-create-and-update" config := fmt.Sprintf(` + resource "github_repository" "%[1]s" { + name = "test-%[2]s" + visibility = "private" + } + resource "github_organization_ruleset" "%[1]s" { name = "test-%[2]s" target = "branch" @@ -72,7 +77,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { do_not_enforce_on_create = true required_workflow { path = "path/to/workflow.yaml" - repository_id = 1234 + repository_id = github_repository.%[1]s.repo_id } } From 6954994ff1f8f56f7e4234d1f0704c0c1e569ff2 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 21:58:52 +0200 Subject: [PATCH 10/72] Actions repository access level needs to be set properly Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 3d93117758..ab5b2c53ed 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -29,6 +29,11 @@ func TestGithubOrganizationRulesets(t *testing.T) { visibility = "private" } + resource "github_actions_repository_access_level" "%[1]s" { + repository = github_repository.%[1]s.name + access_level = "organization" + } + resource "github_organization_ruleset" "%[1]s" { name = "test-%[2]s" target = "branch" From a41b2f38808d45e85c2243fe93b12281273a0b5c Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 21:59:24 +0200 Subject: [PATCH 11/72] Required workflow needs to be in the `.github/workflows` folder 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 ab5b2c53ed..79d56b7092 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -81,7 +81,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { required_workflows { do_not_enforce_on_create = true required_workflow { - path = "path/to/workflow.yaml" + path = ".github/workflows/echo.yaml" repository_id = github_repository.%[1]s.repo_id } } From 41b3deca23cfc3423db073c10fe42739e201ae5c Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 22:00:06 +0200 Subject: [PATCH 12/72] Update indentation Signed-off-by: Timo Sand --- ...esource_github_organization_ruleset_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 79d56b7092..99719bb4bb 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -29,10 +29,10 @@ func TestGithubOrganizationRulesets(t *testing.T) { visibility = "private" } - resource "github_actions_repository_access_level" "%[1]s" { - repository = github_repository.%[1]s.name - access_level = "organization" - } + resource "github_actions_repository_access_level" "%[1]s" { + repository = github_repository.%[1]s.name + access_level = "organization" + } resource "github_organization_ruleset" "%[1]s" { name = "test-%[2]s" @@ -87,11 +87,11 @@ func TestGithubOrganizationRulesets(t *testing.T) { } required_code_scanning { - required_code_scanning_tool { - alerts_threshold = "errors" - security_alerts_threshold = "high_or_higher" - tool = "CodeQL" - } + required_code_scanning_tool { + alerts_threshold = "errors" + security_alerts_threshold = "high_or_higher" + tool = "CodeQL" + } } branch_name_pattern { From 512db50f0c492cc828ecb190692e72da9cbb71e6 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 22:01:30 +0200 Subject: [PATCH 13/72] Need to have one of `repository_name` or `repository_id` defined Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 99719bb4bb..8c23a75fb7 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -236,6 +236,10 @@ func TestGithubOrganizationRulesets(t *testing.T) { include = ["~ALL"] exclude = [] } + repository_name { + include = ["~ALL"] + exclude = [] + } } rules { @@ -334,6 +338,10 @@ func TestGithubOrganizationRulesets(t *testing.T) { include = ["~ALL"] exclude = [] } + repository_name { + include = ["~ALL"] + exclude = [] + } } rules { @@ -440,6 +448,10 @@ func TestGithubOrganizationRulesets(t *testing.T) { include = ["~ALL"] exclude = [] } + repository_name { + include = ["~ALL"] + exclude = [] + } } rules { @@ -527,6 +539,10 @@ func TestGithubOrganizationRulesets(t *testing.T) { include = ["~ALL"] exclude = [] } + repository_name { + include = ["~ALL"] + exclude = [] + } } rules { From bdc47298b05b50422d6796f51d28ff205097b050 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 4 Dec 2025 23:09:19 +0200 Subject: [PATCH 14/72] 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/respository_rules_utils.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/github/respository_rules_utils.go b/github/respository_rules_utils.go index f2c5fdf44d..58a88876a1 100644 --- a/github/respository_rules_utils.go +++ b/github/respository_rules_utils.go @@ -3,11 +3,14 @@ package github import ( "reflect" "sort" + "log" "github.com/google/go-github/v77/github" "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,12 +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{ 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 } @@ -541,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 405d3b628580da4b0e2149946caf45e72be4a882 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:19:04 +0200 Subject: [PATCH 15/72] 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 33efdceb3d..f1c2cab428 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 52c23eba7f..298c4e7362 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -188,6 +188,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 986ae22912eb11d7bb9008dae9181fe07e308325 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:20:18 +0200 Subject: [PATCH 16/72] Fixed type conversion for `allowed_merge_methods` Signed-off-by: Timo Sand --- github/respository_rules_utils.go | 41 +++++++++++++------------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/github/respository_rules_utils.go b/github/respository_rules_utils.go index 58a88876a1..94243bcc2f 100644 --- a/github/respository_rules_utils.go +++ b/github/respository_rules_utils.go @@ -1,9 +1,9 @@ package github import ( + "log" "reflect" "sort" - "log" "github.com/google/go-github/v77/github" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -39,22 +39,19 @@ 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 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 + } + 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 } func resourceGithubRulesetObject(d *schema.ResourceData, org string) *github.RepositoryRuleset { @@ -316,19 +313,15 @@ 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 - } + allowedMergeMethods := pullRequestMap["allowed_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 581ff8c4b0b4eec58544b2d4aa65231ea02da91f Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 5 Dec 2025 00:24:47 +0200 Subject: [PATCH 17/72] Update test content to actually pass the GH API Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 8c23a75fb7..99ba6a41e9 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -27,6 +27,18 @@ func TestGithubOrganizationRulesets(t *testing.T) { resource "github_repository" "%[1]s" { name = "test-%[2]s" visibility = "private" + auto_init = true + lifecycle { prevent_destroy = true } + } + + resource "github_repository_file" "workflow_file" { + repository = github_repository.%[1]s.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" "%[1]s" { @@ -83,6 +95,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { required_workflow { path = ".github/workflows/echo.yaml" repository_id = github_repository.%[1]s.repo_id + ref = "main" # Default ref is master } } @@ -103,6 +116,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { non_fast_forward = true } + depends_on = [github_repository_file.workflow_file] } `, resourceName, randomID) From e97f75c9cb2f8a9b2f800b44b5ec858d49b248b9 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 00:14:00 +0200 Subject: [PATCH 18/72] Convert `github_repository` to use `*Context` CRUD methods This enables better debugging during tests as we are able to use get logs for HTTP req/resp Signed-off-by: Timo Sand --- github/resource_github_repository.go | 92 ++++++++++++++-------------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index 8673877aea..b0791f1217 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -10,18 +10,19 @@ import ( "strings" "github.com/google/go-github/v77/github" + "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" ) func resourceGithubRepository() *schema.Resource { return &schema.Resource{ - Create: resourceGithubRepositoryCreate, - Read: resourceGithubRepositoryRead, - Update: resourceGithubRepositoryUpdate, - Delete: resourceGithubRepositoryDelete, + CreateContext: resourceGithubRepositoryCreate, + ReadContext: resourceGithubRepositoryRead, + UpdateContext: resourceGithubRepositoryUpdate, + DeleteContext: resourceGithubRepositoryDelete, Importer: &schema.ResourceImporter{ - State: func(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { + StateContext: func(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { if err := d.Set("auto_init", false); err != nil { return nil, err } @@ -595,18 +596,17 @@ func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository { return repository } -func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error { +func resourceGithubRepositoryCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client if branchName, hasDefaultBranch := d.GetOk("default_branch"); hasDefaultBranch && (branchName != "main") { - return fmt.Errorf("cannot set the default branch on a new repository to something other than 'main'") + return diag.FromErr(fmt.Errorf("cannot set the default branch on a new repository to something other than 'main'")) } repoReq := resourceGithubRepositoryObject(d) owner := meta.(*Owner).name repoName := repoReq.GetName() - ctx := context.Background() // determine if repository should be private. assume public to start isPrivate := false @@ -632,7 +632,7 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error { for _, templateConfigBlock := range templateConfigBlocks { templateConfigMap, ok := templateConfigBlock.(map[string]any) if !ok { - return errors.New("failed to unpack template configuration block") + return diag.FromErr(errors.New("failed to unpack template configuration block")) } templateRepo := templateConfigMap["repository"].(string) @@ -653,7 +653,7 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error { &templateRepoReq, ) if err != nil { - return err + return diag.FromErr(err) } d.SetId(*repo.Name) @@ -667,7 +667,7 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error { log.Printf("[INFO] Creating fork of %s/%s in %s", sourceOwner, sourceRepo, owner) if sourceOwner == "" || sourceRepo == "" { - return fmt.Errorf("source_owner and source_repo must be provided when forking a repository") + return diag.FromErr(fmt.Errorf("source_owner and source_repo must be provided when forking a repository")) } // Create the fork using the GitHub client library @@ -688,18 +688,18 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error { log.Printf("[INFO] Fork is being created asynchronously") // Despite the 202 status, the API should still return preliminary fork information if fork == nil { - return fmt.Errorf("fork information not available after accepted status") + return diag.FromErr(fmt.Errorf("fork information not available after accepted status")) } log.Printf("[DEBUG] Fork name: %s", fork.GetName()) } else { - return fmt.Errorf("failed to create fork: %w", err) + return diag.FromErr(fmt.Errorf("failed to create fork: %w", err)) } } else if resp != nil { log.Printf("[DEBUG] Fork response status: %d", resp.StatusCode) } if fork == nil { - return fmt.Errorf("fork creation failed - no repository returned") + return diag.FromErr(fmt.Errorf("fork creation failed - no repository returned")) } log.Printf("[INFO] Fork created with name: %s", fork.GetName()) @@ -723,7 +723,7 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error { repo, _, err = client.Repositories.Create(ctx, "", repoReq) } if err != nil { - return err + return diag.FromErr(err) } d.SetId(repo.GetName()) } @@ -732,7 +732,7 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error { if len(topics) > 0 { _, _, err := client.Repositories.ReplaceAllTopics(ctx, owner, repoName, topics) if err != nil { - return err + return diag.FromErr(err) } } @@ -740,19 +740,19 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error { if pages != nil { _, _, err := client.Repositories.EnablePages(ctx, owner, repoName, pages) if err != nil { - return err + return diag.FromErr(err) } } err := updateVulnerabilityAlerts(d, client, ctx, owner, repoName) if err != nil { - return err + return diag.FromErr(err) } - return resourceGithubRepositoryUpdate(d, meta) + return resourceGithubRepositoryUpdate(ctx, d, meta) } -func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error { +func resourceGithubRepositoryRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client owner := meta.(*Owner).name @@ -764,7 +764,7 @@ func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error { owner = explicitOwner } - ctx := context.WithValue(context.Background(), ctxId, d.Id()) + ctx = context.WithValue(ctx, ctxId, d.Id()) if !d.IsNewResource() { ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string)) } @@ -783,7 +783,7 @@ func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error { return nil } } - return err + return diag.FromErr(err) } _ = d.Set("etag", resp.Header.Get("ETag")) @@ -829,10 +829,10 @@ func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error { if repo.GetHasPages() { pages, _, err := client.Repositories.GetPagesInfo(ctx, owner, repoName) if err != nil { - return err + return diag.FromErr(err) } if err := d.Set("pages", flattenPages(pages)); err != nil { - return fmt.Errorf("error setting pages: %w", err) + return diag.FromErr(fmt.Errorf("error setting pages: %w", err)) } } @@ -858,32 +858,32 @@ func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error { "repository": repo.TemplateRepository.Name, }, }); err != nil { - return err + return diag.FromErr(err) } } else { if err = d.Set("template", []any{}); err != nil { - return err + return diag.FromErr(err) } } if !d.Get("ignore_vulnerability_alerts_during_read").(bool) { vulnerabilityAlerts, _, err := client.Repositories.GetVulnerabilityAlerts(ctx, owner, repoName) if err != nil { - return fmt.Errorf("error reading repository vulnerability alerts: %w", err) + return diag.FromErr(fmt.Errorf("error reading repository vulnerability alerts: %w", err)) } if err = d.Set("vulnerability_alerts", vulnerabilityAlerts); err != nil { - return err + return diag.FromErr(err) } } if err = d.Set("security_and_analysis", flattenSecurityAndAnalysis(repo.GetSecurityAndAnalysis())); err != nil { - return err + return diag.FromErr(err) } return nil } -func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error { +func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { // 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") { @@ -913,7 +913,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error { repoName := d.Id() owner := meta.(*Owner).name - ctx := context.WithValue(context.Background(), ctxId, d.Id()) + ctx = context.WithValue(ctx, ctxId, d.Id()) // When the organization has "Require sign off on web-based commits" enabled, // the API doesn't allow you to send `web_commit_signoff_required` in order to @@ -931,7 +931,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error { repo, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) if err != nil { - return err + return diag.FromErr(err) } d.SetId(*repo.Name) @@ -940,7 +940,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error { if opts != nil { pages, res, err := client.Repositories.GetPagesInfo(ctx, owner, repoName) if res.StatusCode != http.StatusNotFound && err != nil { - return err + return diag.FromErr(err) } if pages == nil { @@ -949,12 +949,12 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error { _, err = client.Repositories.UpdatePages(ctx, owner, repoName, opts) } if err != nil { - return err + return diag.FromErr(err) } } else { _, err := client.Repositories.DisablePages(ctx, owner, repoName) if err != nil { - return err + return diag.FromErr(err) } } } @@ -963,7 +963,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error { topics := repoReq.Topics _, _, err = client.Repositories.ReplaceAllTopics(ctx, owner, *repo.Name, topics) if err != nil { - return err + return diag.FromErr(err) } d.SetId(*repo.Name) @@ -971,7 +971,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error { topics := repoReq.Topics _, _, err = client.Repositories.ReplaceAllTopics(ctx, owner, *repo.Name, topics) if err != nil { - return err + return diag.FromErr(err) } } } @@ -979,7 +979,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error { if d.HasChange("vulnerability_alerts") { err = updateVulnerabilityAlerts(d, client, ctx, owner, repoName) if err != nil { - return err + return diag.FromErr(err) } } @@ -990,7 +990,7 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error { _, 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", n.(string))) { - return err + return diag.FromErr(err) } } } else { @@ -1004,21 +1004,21 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta any) error { _, _, err = client.Repositories.Edit(ctx, owner, repoName, repoReq) if err != nil { if !strings.Contains(err.Error(), "422 Privacy is already set") { - return err + return diag.FromErr(err) } } } else { log.Printf("[DEBUG] No privacy update required. private: %v", d.Get("private")) } - return resourceGithubRepositoryRead(d, meta) + return resourceGithubRepositoryRead(ctx, d, meta) } -func resourceGithubRepositoryDelete(d *schema.ResourceData, meta any) error { +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(context.Background(), ctxId, d.Id()) + ctx = context.WithValue(ctx, ctxId, d.Id()) archiveOnDestroy := d.Get("archive_on_destroy").(bool) if archiveOnDestroy { @@ -1027,20 +1027,20 @@ func resourceGithubRepositoryDelete(d *schema.ResourceData, meta any) error { return nil } else { if err := d.Set("archived", true); err != nil { - return err + return diag.FromErr(err) } repoReq := resourceGithubRepositoryObject(d) // Always remove `web_commit_signoff_required` when archiving, to avoid 422 error repoReq.WebCommitSignoffRequired = nil log.Printf("[DEBUG] Archiving repository on delete: %s/%s", owner, repoName) _, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) - return err + return diag.FromErr(err) } } log.Printf("[DEBUG] Deleting repository: %s/%s", owner, repoName) _, err := client.Repositories.Delete(ctx, owner, repoName) - return err + return diag.FromErr(err) } func expandPages(input []any) *github.Pages { From f096f44d74115751b5c64bd70c740578639e04ed Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:17:34 +0200 Subject: [PATCH 19/72] Enable `TestGithubOrganizationRulesets/Creates_and_updates_organization_rulesets_without_errors` to pass Signed-off-by: Timo Sand --- ...esource_github_organization_ruleset_test.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 99ba6a41e9..2eb64ed548 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -28,10 +28,12 @@ func TestGithubOrganizationRulesets(t *testing.T) { name = "test-%[2]s" visibility = "private" auto_init = true - lifecycle { prevent_destroy = true } + + ignore_vulnerability_alerts_during_read = true + } - resource "github_repository_file" "workflow_file" { + resource "github_repository_file" "%[1]s" { repository = github_repository.%[1]s.name branch = "main" file = ".github/workflows/echo.yaml" @@ -73,6 +75,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { required_signatures = false pull_request { + allowed_merge_methods = ["merge", "squash", "rebase"] required_approving_review_count = 2 required_review_thread_resolution = true require_code_owner_review = true @@ -116,7 +119,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { non_fast_forward = true } - depends_on = [github_repository_file.workflow_file] + depends_on = [github_repository_file.%[1]s] } `, resourceName, randomID) @@ -124,7 +127,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { resource.TestCheckResourceAttr( fmt.Sprintf("github_organization_ruleset.%s", resourceName), "name", - "test", + fmt.Sprintf("test-%s", randomID), ), resource.TestCheckResourceAttr( fmt.Sprintf("github_organization_ruleset.%s", resourceName), @@ -139,12 +142,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { resource.TestCheckResourceAttr( fmt.Sprintf("github_organization_ruleset.%s", resourceName), "rules.0.required_workflows.0.required_workflow.0.path", - "path/to/workflow.yaml", - ), - resource.TestCheckResourceAttr( - fmt.Sprintf("github_organization_ruleset.%s", resourceName), - "rules.0.required_workflows.0.required_workflow.0.repository_id", - "1234", + ".github/workflows/echo.yaml", ), resource.TestCheckResourceAttr( fmt.Sprintf("github_organization_ruleset.%s", resourceName), From 2d77c1e77244e50e3cdbbe82192e7c304ba17140 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 19:58:45 +0200 Subject: [PATCH 20/72] `make fmt` Signed-off-by: Timo Sand --- github/respository_rules_utils.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/github/respository_rules_utils.go b/github/respository_rules_utils.go index 94243bcc2f..ebe06d0fed 100644 --- a/github/respository_rules_utils.go +++ b/github/respository_rules_utils.go @@ -536,14 +536,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) @@ -564,7 +564,7 @@ func flattenRules(rules *github.RepositoryRulesetRules, org bool) []any { "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) + log.Printf("[DEBUG] Flattened Pull Request rules slice request slice: %#v", pullRequestSlice) rulesMap["pull_request"] = pullRequestSlice } From 9d22505853a851df0bb542e4b5e616602a3a6304 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:59:04 +0200 Subject: [PATCH 21/72] Add workaround for GH API bug that OrgAdmin actor_id is returned as `null` Signed-off-by: Timo Sand --- github/respository_rules_utils.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/github/respository_rules_utils.go b/github/respository_rules_utils.go index ebe06d0fed..8b1d788604 100644 --- a/github/respository_rules_utils.go +++ b/github/respository_rules_utils.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 74bcfe745f5023168818dcd3fa03fe0a60947ffb Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 20:35:11 +0200 Subject: [PATCH 22/72] `make fmt` Signed-off-by: Timo Sand --- github/respository_rules_utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/respository_rules_utils.go b/github/respository_rules_utils.go index 8b1d788604..d90ef5b868 100644 --- a/github/respository_rules_utils.go +++ b/github/respository_rules_utils.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 480848399461f3caa59a645be42cfb1f566ddc4a Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 02:01:15 +0200 Subject: [PATCH 23/72] 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 --- .../resource_github_organization_ruleset.go | 2 +- ...source_github_organization_ruleset_test.go | 24 ++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index f1c2cab428..6d58db82c0 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.", diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 2eb64ed548..651d9b7212 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -322,8 +322,9 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) t.Run("Creates and updates organization using bypasses", func(t *testing.T) { + resourceName := "test-creates-and-updates-using-bypasses" config := fmt.Sprintf(` - resource "github_organization_ruleset" "test" { + resource "github_organization_ruleset" "%s" { name = "test-%s" target = "branch" enforcement = "active" @@ -363,6 +364,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { required_linear_history = true required_signatures = false pull_request { + allowed_merge_methods = ["merge", "squash", "rebase"] required_approving_review_count = 2 required_review_thread_resolution = true require_code_owner_review = true @@ -371,43 +373,43 @@ func TestGithubOrganizationRulesets(t *testing.T) { } } } - `, randomID) + `, resourceName, randomID) check := resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.#", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.#", "3", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.0.actor_type", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.0.actor_type", "DeployKey", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.0.bypass_mode", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.0.bypass_mode", "always", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.1.actor_id", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.actor_id", "5", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.1.actor_type", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.actor_type", "RepositoryRole", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.1.bypass_mode", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.bypass_mode", "always", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.2.actor_id", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.actor_id", "1", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.2.actor_type", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.actor_type", "OrganizationAdmin", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.2.bypass_mode", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.bypass_mode", "always", ), ) From f698cd2c29c984ecd215ae4ac950d2fa55b7372d Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 02:13:49 +0200 Subject: [PATCH 24/72] Fix tests regarding `bypass_actors` ordering Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 651d9b7212..20a67b11d7 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -389,27 +389,27 @@ func TestGithubOrganizationRulesets(t *testing.T) { "always", ), resource.TestCheckResourceAttr( - fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.actor_id", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.actor_id", "5", ), resource.TestCheckResourceAttr( - fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.actor_type", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.actor_type", "RepositoryRole", ), resource.TestCheckResourceAttr( - fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.bypass_mode", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.bypass_mode", "always", ), resource.TestCheckResourceAttr( - fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.actor_id", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.actor_id", "1", ), resource.TestCheckResourceAttr( - fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.actor_type", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.actor_type", "OrganizationAdmin", ), resource.TestCheckResourceAttr( - fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.bypass_mode", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.bypass_mode", "always", ), ) @@ -433,8 +433,9 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) t.Run("Creates organization ruleset with all bypass_modes", func(t *testing.T) { + resourceName := "test-create-with-bypass-modes" config := fmt.Sprintf(` - resource "github_organization_ruleset" "test" { + resource "github_organization_ruleset" "%s" { name = "test-bypass-modes-%s" target = "branch" enforcement = "active" @@ -472,47 +473,47 @@ func TestGithubOrganizationRulesets(t *testing.T) { creation = true } } - `, randomID) + `, resourceName, randomID) check := resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.#", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.#", "3", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.0.actor_id", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.0.actor_id", "1", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.0.actor_type", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.0.actor_type", "OrganizationAdmin", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.0.bypass_mode", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.0.bypass_mode", "always", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.1.actor_id", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.actor_id", "5", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.1.actor_type", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.actor_type", "RepositoryRole", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.1.bypass_mode", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.2.bypass_mode", "pull_request", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.2.actor_id", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.actor_id", "2", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.2.actor_type", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.actor_type", "RepositoryRole", ), resource.TestCheckResourceAttr( - "github_organization_ruleset.test", "bypass_actors.2.bypass_mode", + fmt.Sprintf("github_organization_ruleset.%s", resourceName), "bypass_actors.1.bypass_mode", "exempt", ), ) From 3752c1631055de917d374d23eb2848a9c3be3f3c Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 02:14:07 +0200 Subject: [PATCH 25/72] Rename test resources for easier debugging Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 20a67b11d7..60754d58ba 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -237,8 +237,9 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) t.Run("Imports rulesets without error", func(t *testing.T) { + resourceName := "test-import-rulesets" config := fmt.Sprintf(` - resource "github_organization_ruleset" "test" { + resource "github_organization_ruleset" "%s" { name = "test-%s" target = "branch" enforcement = "active" @@ -265,6 +266,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { required_signatures = false pull_request { + allowed_merge_methods = ["merge", "squash", "rebase"] required_approving_review_count = 2 required_review_thread_resolution = true require_code_owner_review = true @@ -292,10 +294,10 @@ func TestGithubOrganizationRulesets(t *testing.T) { non_fast_forward = true } } - `, randomID) + `, resourceName, randomID) check := resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrSet("github_organization_ruleset.test", "name"), + resource.TestCheckResourceAttrSet(fmt.Sprintf("github_organization_ruleset.%s", resourceName), "name"), ) testCase := func(t *testing.T, mode string) { @@ -308,7 +310,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { Check: check, }, { - ResourceName: "github_organization_ruleset.test", + ResourceName: fmt.Sprintf("github_organization_ruleset.%s", resourceName), ImportState: true, ImportStateVerify: true, }, From cf35bf95309a20767460068177c2180f21e80525 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 27 Nov 2025 22:11:15 +0200 Subject: [PATCH 26/72] Update descriptions and validations Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 6d58db82c0..8bd70079b9 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -36,8 +36,8 @@ func resourceGithubOrganizationRuleset() *schema.Resource { "target": { Type: schema.TypeString, Required: true, - ValidateFunc: validation.StringInSlice([]string{"branch", "tag", "push"}, false), - Description: "Possible values are `branch`, `tag` and `push`. Note: The `push` target is in beta and is subject to change.", + ValidateFunc: validation.StringInSlice([]string{"branch", "tag", "push", "repository"}, false), + Description: "The target of the ruleset. Possible values are `branch`, `tag`, `push` and `repository`.", }, "enforcement": { Type: schema.TypeString, @@ -87,7 +87,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Type: schema.TypeList, Optional: true, MaxItems: 1, - Description: "Parameters for an organization ruleset condition. `ref_name` is required alongside one of `repository_name` or `repository_id`.", + Description: "Parameters for an organization ruleset condition. For `branch` and `tag` targets, `ref_name` is required alongside one of `repository_name` or `repository_id`. The `push` target does not require `ref_name` conditions. For `repository` target, the conditions object should only contain the `repository_name` and the `repository_id`.", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "ref_name": { From 35ff66870501e95da292ec64053aa2a062ed15ee Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 27 Nov 2025 22:11:35 +0200 Subject: [PATCH 27/72] Add `CustomizeDiff` logic to validate on `plan` Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 8bd70079b9..95f8f2e541 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -26,6 +26,8 @@ func resourceGithubOrganizationRuleset() *schema.Resource { SchemaVersion: 1, + CustomizeDiff: validateConditionsFieldBasedOnTarget, + Schema: map[string]*schema.Schema{ "name": { Type: schema.TypeString, @@ -729,3 +731,47 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso return []*schema.ResourceData{d}, nil } + + +func validateConditionsFieldForBranchAndTagTargets(d *schema.ResourceDiff, meta interface{}) error { + conditions := d.Get("conditions").([]any)[0].(map[string]any) + if conditions["ref_name"] == nil || conditions["repository_name"] == nil || conditions["repository_id"] == nil { + return fmt.Errorf("ref_name and repository_name or ref_name and repository_id must be set for branch and tag targets") + } + return nil +} + +func validateConditionsFieldForPushTarget(d *schema.ResourceDiff, meta interface{}) error { + conditions := d.Get("conditions").([]any)[0].(map[string]any) + if conditions["ref_name"] != nil { + return fmt.Errorf("ref_name must not be set for push target") + } + return nil +} + +func validateConditionsFieldForRepositoryTarget(d *schema.ResourceDiff, meta interface{}) error { + conditions := d.Get("conditions").([]any)[0].(map[string]any) + if conditions["ref_name"] != nil { + return fmt.Errorf("ref_name must not be set for repository target") + } + if conditions["repository_name"] == nil && conditions["repository_id"] == nil { + return fmt.Errorf("repository_name or repository_id must be set for repository target") + } + return nil +} + + +func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + target := d.Get("target").(string) + conditions := d.Get("conditions").([]any)[0].(map[string]any) + + switch target { + case "branch", "tag": + return validateConditionsFieldForBranchAndTagTargets(d, meta) + case "push": + return validateConditionsFieldForPushTarget(d, meta) + case "repository": + return validateConditionsFieldForRepositoryTarget(d, meta) + } + return nil +} From a944da8e60733343d759bf79245a0e67f273b396 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 29 Nov 2025 19:42:07 +0200 Subject: [PATCH 28/72] Allow `organization` in tests as well Signed-off-by: Timo Sand --- github/provider_utils.go | 4 ++-- github/resource_github_organization_ruleset_test.go | 8 -------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/github/provider_utils.go b/github/provider_utils.go index b7b35561b9..36e976f254 100644 --- a/github/provider_utils.go +++ b/github/provider_utils.go @@ -67,10 +67,10 @@ func skipUnlessMode(t *testing.T, providerMode string) { t.Log("GITHUB_TOKEN and GITHUB_OWNER environment variables should be set") } case organization: - if os.Getenv("GITHUB_TOKEN") != "" && os.Getenv("GITHUB_ORGANIZATION") != "" { + if os.Getenv("GITHUB_TOKEN") != "" && (os.Getenv("GITHUB_ORGANIZATION") != "" || os.Getenv("GITHUB_TEST_ORGANIZATION") != "") { return } else { - t.Log("GITHUB_TOKEN and GITHUB_ORGANIZATION environment variables should be set") + t.Log("GITHUB_TOKEN and GITHUB_ORGANIZATION or GITHUB_TEST_ORGANIZATION environment variables should be set") } } diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 60754d58ba..dff70eea9c 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -11,14 +11,6 @@ import ( ) func TestGithubOrganizationRulesets(t *testing.T) { - if isEnterprise != "true" { - t.Skip("Skipping because `ENTERPRISE_ACCOUNT` is not set or set to false") - } - - if testEnterprise == "" { - t.Skip("Skipping because `ENTERPRISE_SLUG` is not set") - } - randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) t.Run("Creates and updates organization rulesets without errors", func(t *testing.T) { From e471a587a0eb40511294bbe62f8503c2b305fd33 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 29 Nov 2025 19:42:18 +0200 Subject: [PATCH 29/72] Remove unused leftover Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 1 - 1 file changed, 1 deletion(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 95f8f2e541..df245c7fca 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -763,7 +763,6 @@ func validateConditionsFieldForRepositoryTarget(d *schema.ResourceDiff, meta int func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { target := d.Get("target").(string) - conditions := d.Get("conditions").([]any)[0].(map[string]any) switch target { case "branch", "tag": From 92fc53849891d2554f7a71ece0260e25862c57e0 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 29 Nov 2025 19:44:16 +0200 Subject: [PATCH 30/72] Add first validation test Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index dff70eea9c..1930d938d5 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -2,6 +2,7 @@ package github import ( "fmt" + "regexp" "strings" "testing" @@ -603,6 +604,57 @@ func TestGithubOrganizationRulesets(t *testing.T) { testCase(t, enterprise) }) }) + + t.Run("Validates branch target requires ref_name", func(t *testing.T) { + config := fmt.Sprintf(` + resource "github_organization_ruleset" "test" { + name = "test-validation-%s" + target = "branch" + enforcement = "active" + + conditions { + repository_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + creation = true + } + } + `, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("Insufficient ref_name blocks"), + // ExpectError: regexp.MustCompile("ref_name and repository_name or ref_name and repository_id must be set for branch and tag targets"), + }, + }, + }) + } + + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) + }) + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + t.Skip("individual account not supported for this operation") + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) } func TestOrganizationPushRulesetSupport(t *testing.T) { From 4f2ea6adf075429d343f22e9599192f96682dd75 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 30 Nov 2025 08:11:54 +0200 Subject: [PATCH 31/72] Update formatting Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 62 +++++++++---------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index df245c7fca..bd8443febc 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -26,7 +26,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { SchemaVersion: 1, - CustomizeDiff: validateConditionsFieldBasedOnTarget, + CustomizeDiff: validateConditionsFieldBasedOnTarget, Schema: map[string]*schema.Schema{ "name": { @@ -732,45 +732,43 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso return []*schema.ResourceData{d}, nil } - func validateConditionsFieldForBranchAndTagTargets(d *schema.ResourceDiff, meta interface{}) error { - conditions := d.Get("conditions").([]any)[0].(map[string]any) - if conditions["ref_name"] == nil || conditions["repository_name"] == nil || conditions["repository_id"] == nil { - return fmt.Errorf("ref_name and repository_name or ref_name and repository_id must be set for branch and tag targets") - } - return nil + conditions := d.Get("conditions").([]any)[0].(map[string]any) + if conditions["ref_name"] == nil || conditions["repository_name"] == nil || conditions["repository_id"] == nil { + return fmt.Errorf("ref_name and repository_name or ref_name and repository_id must be set for branch and tag targets") + } + return nil } func validateConditionsFieldForPushTarget(d *schema.ResourceDiff, meta interface{}) error { - conditions := d.Get("conditions").([]any)[0].(map[string]any) - if conditions["ref_name"] != nil { - return fmt.Errorf("ref_name must not be set for push target") - } - return nil + conditions := d.Get("conditions").([]any)[0].(map[string]any) + if conditions["ref_name"] != nil { + return fmt.Errorf("ref_name must not be set for push target") + } + return nil } func validateConditionsFieldForRepositoryTarget(d *schema.ResourceDiff, meta interface{}) error { - conditions := d.Get("conditions").([]any)[0].(map[string]any) - if conditions["ref_name"] != nil { - return fmt.Errorf("ref_name must not be set for repository target") - } - if conditions["repository_name"] == nil && conditions["repository_id"] == nil { - return fmt.Errorf("repository_name or repository_id must be set for repository target") - } - return nil + conditions := d.Get("conditions").([]any)[0].(map[string]any) + if conditions["ref_name"] != nil { + return fmt.Errorf("ref_name must not be set for repository target") + } + if conditions["repository_name"] == nil && conditions["repository_id"] == nil { + return fmt.Errorf("repository_name or repository_id must be set for repository target") + } + return nil } - func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { - target := d.Get("target").(string) - - switch target { - case "branch", "tag": - return validateConditionsFieldForBranchAndTagTargets(d, meta) - case "push": - return validateConditionsFieldForPushTarget(d, meta) - case "repository": - return validateConditionsFieldForRepositoryTarget(d, meta) - } - return nil + target := d.Get("target").(string) + + switch target { + case "branch", "tag": + return validateConditionsFieldForBranchAndTagTargets(d, meta) + case "push": + return validateConditionsFieldForPushTarget(d, meta) + case "repository": + return validateConditionsFieldForRepositoryTarget(d, meta) + } + return nil } From 0e81c85c90563e0ca37800b26404fe0fd975609e Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 30 Nov 2025 08:22:36 +0200 Subject: [PATCH 32/72] Add validation error when `conditions` is missing Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 6 ++++ ...source_github_organization_ruleset_test.go | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index bd8443febc..d4e0c765e3 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -761,6 +761,12 @@ func validateConditionsFieldForRepositoryTarget(d *schema.ResourceDiff, meta int func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { target := d.Get("target").(string) + conditionsRaw := d.Get("conditions").([]any) + + // Handle empty conditions - branch and tag targets require conditions with ref_name + if len(conditionsRaw) == 0 { + return fmt.Errorf("conditions block is required for %s target", target) + } switch target { case "branch", "tag": diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 1930d938d5..90437880d9 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -605,6 +605,39 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) }) + t.Run("panics when conditions block is missing for branch target", func(t *testing.T) { + config := fmt.Sprintf(` + resource "github_organization_ruleset" "test" { + name = "test-no-conditions-%s" + target = "branch" + enforcement = "active" + + rules { + creation = true + } + } + `, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + // Before fix: This will PANIC with "index out of range" + // After fix: Should return proper validation error + ExpectError: regexp.MustCompile(`conditions block is required for branch target`), + }, + }, + }) + } + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) + t.Run("Validates branch target requires ref_name", func(t *testing.T) { config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { From 056a70f5764bee60ad129ea1b187511983d63ec2 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 30 Nov 2025 09:23:50 +0200 Subject: [PATCH 33/72] Add further validation tests Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 90437880d9..5330a93af3 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -688,6 +688,157 @@ func TestGithubOrganizationRulesets(t *testing.T) { testCase(t, organization) }) }) + + t.Run("Validates tag target requires conditions", func(t *testing.T) { + config := fmt.Sprintf(` + resource "github_organization_ruleset" "test" { + name = "test-tag-no-conditions-%s" + target = "tag" + enforcement = "active" + + rules { + creation = true + } + } + `, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`conditions block is required for tag target`), + }, + }, + }) + } + + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) + }) + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + t.Skip("individual account not supported for this operation") + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) + + t.Run("Validates push target rejects ref_name", func(t *testing.T) { + config := fmt.Sprintf(` + resource "github_organization_ruleset" "test" { + name = "test-push-with-ref-%s" + target = "push" + enforcement = "active" + + conditions { + ref_name { + include = ["~ALL"] + exclude = [] + } + repository_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + creation = true + } + } + `, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`ref_name must not be set for push target`), + }, + }, + }) + } + + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) + }) + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + t.Skip("individual account not supported for this operation") + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) + + t.Run("Validates repository target rejects ref_name", func(t *testing.T) { + config := fmt.Sprintf(` + resource "github_organization_ruleset" "test" { + name = "test-repo-with-ref-%s" + target = "repository" + enforcement = "active" + + conditions { + ref_name { + include = ["~ALL"] + exclude = [] + } + repository_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + creation = true + } + } + `, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`ref_name must not be set for repository target`), + }, + }, + }) + } + + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) + }) + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + t.Skip("individual account not supported for this operation") + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) } func TestOrganizationPushRulesetSupport(t *testing.T) { From 740c5e29de435cbf9a93062693b759795961d7e2 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 2 Dec 2025 21:55:23 +0200 Subject: [PATCH 34/72] Remove unnecessary skip blocks as `individual` and `anonymous` access to org rulesets will never be a thing Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 40 ++----------------- 1 file changed, 4 insertions(+), 36 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 5330a93af3..9f97c3ddab 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -676,16 +676,8 @@ func TestGithubOrganizationRulesets(t *testing.T) { testCase(t, enterprise) }) - t.Run("with an anonymous account", func(t *testing.T) { - t.Skip("anonymous account not supported for this operation") - }) - - t.Run("with an individual account", func(t *testing.T) { - t.Skip("individual account not supported for this operation") - }) - t.Run("with an organization account", func(t *testing.T) { - testCase(t, organization) + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") }) }) @@ -719,16 +711,8 @@ func TestGithubOrganizationRulesets(t *testing.T) { testCase(t, enterprise) }) - t.Run("with an anonymous account", func(t *testing.T) { - t.Skip("anonymous account not supported for this operation") - }) - - t.Run("with an individual account", func(t *testing.T) { - t.Skip("individual account not supported for this operation") - }) - t.Run("with an organization account", func(t *testing.T) { - testCase(t, organization) + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") }) }) @@ -773,16 +757,8 @@ func TestGithubOrganizationRulesets(t *testing.T) { testCase(t, enterprise) }) - t.Run("with an anonymous account", func(t *testing.T) { - t.Skip("anonymous account not supported for this operation") - }) - - t.Run("with an individual account", func(t *testing.T) { - t.Skip("individual account not supported for this operation") - }) - t.Run("with an organization account", func(t *testing.T) { - testCase(t, organization) + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") }) }) @@ -827,16 +803,8 @@ func TestGithubOrganizationRulesets(t *testing.T) { testCase(t, enterprise) }) - t.Run("with an anonymous account", func(t *testing.T) { - t.Skip("anonymous account not supported for this operation") - }) - - t.Run("with an individual account", func(t *testing.T) { - t.Skip("individual account not supported for this operation") - }) - t.Run("with an organization account", func(t *testing.T) { - testCase(t, organization) + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") }) }) } From 4fc2f514b9f417d3684dfb6ca547e74c1e663844 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 2 Dec 2025 21:57:29 +0200 Subject: [PATCH 35/72] Switch `ref_name` to `Optional` as `push` doesn't need a `ref_name` Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 2 +- github/resource_github_organization_ruleset_test.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index d4e0c765e3..a7cfbbe236 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -94,7 +94,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Schema: map[string]*schema.Schema{ "ref_name": { Type: schema.TypeList, - Required: true, + Optional: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 9f97c3ddab..637e398802 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -665,8 +665,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile("Insufficient ref_name blocks"), - // ExpectError: regexp.MustCompile("ref_name and repository_name or ref_name and repository_id must be set for branch and tag targets"), + ExpectError: regexp.MustCompile("ref_name and repository_name or ref_name and repository_id must be set for branch and tag targets"), }, }, }) From a70831e4655965687d4b1b22089da45c028ae430 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 12:34:24 +0200 Subject: [PATCH 36/72] Add Debug logging with `tflog` to validation Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index a7cfbbe236..f7b3bb60f7 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -12,6 +12,7 @@ import ( "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" + "github.com/hashicorp/terraform-plugin-log/tflog" ) func resourceGithubOrganizationRuleset() *schema.Resource { @@ -732,25 +733,26 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso return []*schema.ResourceData{d}, nil } -func validateConditionsFieldForBranchAndTagTargets(d *schema.ResourceDiff, meta interface{}) error { +func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { conditions := d.Get("conditions").([]any)[0].(map[string]any) - if conditions["ref_name"] == nil || conditions["repository_name"] == nil || conditions["repository_id"] == nil { - return fmt.Errorf("ref_name and repository_name or ref_name and repository_id must be set for branch and tag targets") + tflog.Debug(ctx, "Validating conditions field for branch and tag targets", map[string]interface{}{"conditions": conditions}) } return nil } -func validateConditionsFieldForPushTarget(d *schema.ResourceDiff, meta interface{}) error { +func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { conditions := d.Get("conditions").([]any)[0].(map[string]any) - if conditions["ref_name"] != nil { + tflog.Debug(ctx, "Validating conditions field for push target", map[string]interface{}{"conditions": conditions}) + return fmt.Errorf("ref_name must not be set for push target") } return nil } -func validateConditionsFieldForRepositoryTarget(d *schema.ResourceDiff, meta interface{}) error { +func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { conditions := d.Get("conditions").([]any)[0].(map[string]any) - if conditions["ref_name"] != nil { + tflog.Debug(ctx, "Validating conditions field for repository target", map[string]interface{}{"conditions": conditions}) + return fmt.Errorf("ref_name must not be set for repository target") } if conditions["repository_name"] == nil && conditions["repository_id"] == nil { @@ -761,6 +763,7 @@ func validateConditionsFieldForRepositoryTarget(d *schema.ResourceDiff, meta int func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { target := d.Get("target").(string) + tflog.Debug(ctx, "Validating conditions field based on target", map[string]interface{}{"target": target}) conditionsRaw := d.Get("conditions").([]any) // Handle empty conditions - branch and tag targets require conditions with ref_name @@ -770,11 +773,11 @@ func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.Resourc switch target { case "branch", "tag": - return validateConditionsFieldForBranchAndTagTargets(d, meta) + return validateConditionsFieldForBranchAndTagTargets(ctx, d, meta) case "push": - return validateConditionsFieldForPushTarget(d, meta) + return validateConditionsFieldForPushTarget(ctx, d, meta) case "repository": - return validateConditionsFieldForRepositoryTarget(d, meta) + return validateConditionsFieldForRepositoryTarget(ctx, d, meta) } return nil } From 9d8573425ce4daf8b57bc8e85d04de71cccd5299 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 12:35:10 +0200 Subject: [PATCH 37/72] Fix validation as `ref_name`, `repository_name` and `repository_id` are empty lists by default Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index f7b3bb60f7..566973c98d 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -736,6 +736,11 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { conditions := d.Get("conditions").([]any)[0].(map[string]any) tflog.Debug(ctx, "Validating conditions field for branch and tag targets", map[string]interface{}{"conditions": conditions}) + if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 { + return fmt.Errorf("ref_name must be set for branch and tag targets") + } + if conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0 || conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0 { + return fmt.Errorf("Either repository_name or repository_id must be set for branch and tag targets") } return nil } @@ -744,6 +749,7 @@ func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.Resourc conditions := d.Get("conditions").([]any)[0].(map[string]any) tflog.Debug(ctx, "Validating conditions field for push target", map[string]interface{}{"conditions": conditions}) + if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { return fmt.Errorf("ref_name must not be set for push target") } return nil @@ -753,9 +759,10 @@ func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.R conditions := d.Get("conditions").([]any)[0].(map[string]any) tflog.Debug(ctx, "Validating conditions field for repository target", map[string]interface{}{"conditions": conditions}) + if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { return fmt.Errorf("ref_name must not be set for repository target") } - if conditions["repository_name"] == nil && conditions["repository_id"] == nil { + if conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0 || conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0 { return fmt.Errorf("repository_name or repository_id must be set for repository target") } return nil From 5b88bb28643bde2f102d586750f18e599561d3f7 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 12:35:31 +0200 Subject: [PATCH 38/72] Correctly use `enterprise` runner Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 637e398802..abca608091 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -633,8 +633,12 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) } + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) + }) + t.Run("with an organization account", func(t *testing.T) { - testCase(t, organization) + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") }) }) From 08a73fc3f59102c834ed2e05d13f50342fb0efe3 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 12:35:41 +0200 Subject: [PATCH 39/72] Fix test output expectation 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 abca608091..5121551bf4 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -669,7 +669,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile("ref_name and repository_name or ref_name and repository_id must be set for branch and tag targets"), + ExpectError: regexp.MustCompile("ref_name must be set for branch and tag targets"), }, }, }) From 3f34fb646704a6909f307ad2ae7326258b16474b Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 12:49:48 +0200 Subject: [PATCH 40/72] Remove unnecessary panic test Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 6 +-- ...source_github_organization_ruleset_test.go | 37 ------------------- 2 files changed, 3 insertions(+), 40 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 566973c98d..185337349d 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -773,9 +773,9 @@ func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.Resourc tflog.Debug(ctx, "Validating conditions field based on target", map[string]interface{}{"target": target}) conditionsRaw := d.Get("conditions").([]any) - // Handle empty conditions - branch and tag targets require conditions with ref_name - if len(conditionsRaw) == 0 { - return fmt.Errorf("conditions block is required for %s target", target) + if conditionsRaw == nil || len(conditionsRaw) == 0 { + tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]interface{}{"target": target}) + return nil } switch target { diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 5121551bf4..e35aaec7ae 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -605,43 +605,6 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) }) - t.Run("panics when conditions block is missing for branch target", func(t *testing.T) { - config := fmt.Sprintf(` - resource "github_organization_ruleset" "test" { - name = "test-no-conditions-%s" - target = "branch" - enforcement = "active" - - rules { - creation = true - } - } - `, randomID) - - testCase := func(t *testing.T, mode string) { - resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessMode(t, mode) }, - Providers: testAccProviders, - Steps: []resource.TestStep{ - { - Config: config, - // Before fix: This will PANIC with "index out of range" - // After fix: Should return proper validation error - ExpectError: regexp.MustCompile(`conditions block is required for branch target`), - }, - }, - }) - } - - t.Run("with an enterprise account", func(t *testing.T) { - testCase(t, enterprise) - }) - - t.Run("with an organization account", func(t *testing.T) { - t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") - }) - }) - t.Run("Validates branch target requires ref_name", func(t *testing.T) { config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { From 776fc64f0cf4b4a093ae787d1ec7bab2ede87927 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 13:00:34 +0200 Subject: [PATCH 41/72] Improve validation output messages Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 19 +++++++++++-------- ...source_github_organization_ruleset_test.go | 19 +++++++++++++------ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 185337349d..1051b9da9c 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -734,36 +734,39 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso } func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + target := d.Get("target").(string) conditions := d.Get("conditions").([]any)[0].(map[string]any) - tflog.Debug(ctx, "Validating conditions field for branch and tag targets", map[string]interface{}{"conditions": conditions}) + tflog.Debug(ctx, "Validating conditions field for branch and tag targets", map[string]interface{}{"target": target, "conditions": conditions}) if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 { - return fmt.Errorf("ref_name must be set for branch and tag targets") + return fmt.Errorf("ref_name must be set for %s target", target) } if conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0 || conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0 { - return fmt.Errorf("Either repository_name or repository_id must be set for branch and tag targets") + return fmt.Errorf("Either repository_name or repository_id must be set for %s target", target) } return nil } func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + target := d.Get("target").(string) conditions := d.Get("conditions").([]any)[0].(map[string]any) - tflog.Debug(ctx, "Validating conditions field for push target", map[string]interface{}{"conditions": conditions}) + tflog.Debug(ctx, "Validating conditions field for push target", map[string]interface{}{"target": target, "conditions": conditions}) if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { - return fmt.Errorf("ref_name must not be set for push target") + return fmt.Errorf("ref_name must not be set for %s target", target) } return nil } func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + target := d.Get("target").(string) conditions := d.Get("conditions").([]any)[0].(map[string]any) - tflog.Debug(ctx, "Validating conditions field for repository target", map[string]interface{}{"conditions": conditions}) + tflog.Debug(ctx, "Validating conditions field for repository target", map[string]interface{}{"target": target, "conditions": conditions}) if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { - return fmt.Errorf("ref_name must not be set for repository target") + return fmt.Errorf("ref_name must not be set for %s target", target) } if conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0 || conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0 { - return fmt.Errorf("repository_name or repository_id must be set for repository target") + return fmt.Errorf("repository_name or repository_id must be set for %s target", target) } return nil } diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index e35aaec7ae..0bb29e44cb 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -605,7 +605,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) }) - t.Run("Validates branch target requires ref_name", func(t *testing.T) { + t.Run("Validates branch target requires `ref_name` condition", func(t *testing.T) { config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { name = "test-validation-%s" @@ -632,7 +632,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile("ref_name must be set for branch and tag targets"), + ExpectError: regexp.MustCompile("ref_name must be set for branch target"), }, }, }) @@ -647,13 +647,20 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) }) - t.Run("Validates tag target requires conditions", func(t *testing.T) { + t.Run("Validates tag target requires `ref_name` condition", func(t *testing.T) { config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { name = "test-tag-no-conditions-%s" target = "tag" enforcement = "active" + conditions { + repository_name { + include = ["~ALL"] + exclude = [] + } + } + rules { creation = true } @@ -667,7 +674,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile(`conditions block is required for tag target`), + ExpectError: regexp.MustCompile("ref_name must be set for tag target"), }, }, }) @@ -713,7 +720,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile(`ref_name must not be set for push target`), + ExpectError: regexp.MustCompile("ref_name must not be set for push target"), }, }, }) @@ -759,7 +766,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile(`ref_name must not be set for repository target`), + ExpectError: regexp.MustCompile("ref_name must not be set for repository target"), }, }, }) From 7f655080138eed3c23ee53227889ac0977440223 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 14:45:36 +0200 Subject: [PATCH 42/72] Update to use Context aware CRUD functions By using the `*Context` CRUD functions we get context pass-through and enable HTTP Req/Resp Debug logging Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 1051b9da9c..eaf792e956 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -9,10 +9,10 @@ import ( "strconv" "github.com/google/go-github/v77/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" - "github.com/hashicorp/terraform-plugin-log/tflog" ) func resourceGithubOrganizationRuleset() *schema.Resource { @@ -733,10 +733,10 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso return []*schema.ResourceData{d}, nil } -func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { - target := d.Get("target").(string) +func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, meta any) error { + target := d.Get("target").(string) conditions := d.Get("conditions").([]any)[0].(map[string]any) - tflog.Debug(ctx, "Validating conditions field for branch and tag targets", map[string]interface{}{"target": target, "conditions": conditions}) + tflog.Debug(ctx, "Validating conditions field for branch and tag targets", map[string]any{"target": target, "conditions": conditions}) if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 { return fmt.Errorf("ref_name must be set for %s target", target) } @@ -746,10 +746,10 @@ func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schem return nil } -func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { - target := d.Get("target").(string) +func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { + target := d.Get("target").(string) conditions := d.Get("conditions").([]any)[0].(map[string]any) - tflog.Debug(ctx, "Validating conditions field for push target", map[string]interface{}{"target": target, "conditions": conditions}) + tflog.Debug(ctx, "Validating conditions field for push target", map[string]any{"target": target, "conditions": conditions}) if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { return fmt.Errorf("ref_name must not be set for %s target", target) @@ -757,12 +757,12 @@ func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.Resourc return nil } -func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { - target := d.Get("target").(string) +func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { + target := d.Get("target").(string) conditions := d.Get("conditions").([]any)[0].(map[string]any) - tflog.Debug(ctx, "Validating conditions field for repository target", map[string]interface{}{"target": target, "conditions": conditions}) + tflog.Debug(ctx, "Validating conditions field for repository target", map[string]any{"target": target, "conditions": conditions}) - if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { + if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { return fmt.Errorf("ref_name must not be set for %s target", target) } if conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0 || conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0 { @@ -771,13 +771,13 @@ func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.R return nil } -func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { +func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { target := d.Get("target").(string) - tflog.Debug(ctx, "Validating conditions field based on target", map[string]interface{}{"target": target}) + tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target}) conditionsRaw := d.Get("conditions").([]any) - if conditionsRaw == nil || len(conditionsRaw) == 0 { - tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]interface{}{"target": target}) + if conditionsRaw == nil || len(conditionsRaw) == 0 { + tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]any{"target": target}) return nil } From 2479b73837786c1407c79b206e02423e48e2e3cb Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 17:12:51 +0200 Subject: [PATCH 43/72] Fix condition to require only one of `repository_name` or `repository_id` 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 eaf792e956..c7d7b8064f 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -740,7 +740,7 @@ func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schem if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 { return fmt.Errorf("ref_name must be set for %s target", target) } - if conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0 || conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0 { + if (conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0) && (conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0){ return fmt.Errorf("Either repository_name or repository_id must be set for %s target", target) } return nil From 4666bbe365c4a45569c393a8fb59ab5128710ab9 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:21:25 +0200 Subject: [PATCH 44/72] Add validation to `required_workflow.path` Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index c7d7b8064f..ce932c10bd 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -6,6 +6,7 @@ import ( "fmt" "log" "net/http" + "regexp" "strconv" "github.com/google/go-github/v77/github" @@ -468,9 +469,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "The repository in which the workflow is defined.", }, "path": { - Type: schema.TypeString, - Required: true, - Description: "The path to the workflow YAML definition file.", + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: toDiagFunc(validation.StringMatch(regexp.MustCompile(`^\.github\/workflows\/.*$`), "Path must be in the .github/workflows directory"), "path"), + Description: "The path to the workflow YAML definition file.", }, "ref": { Type: schema.TypeString, From e9c10b474c3322dd384fac1423afad98031ff248 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:22:37 +0200 Subject: [PATCH 45/72] `make fmt` Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 2 +- github/respository_rules_utils.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index ce932c10bd..415d55543c 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -742,7 +742,7 @@ func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schem if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 { return fmt.Errorf("ref_name must be set for %s target", target) } - if (conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0) && (conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0){ + if (conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0) && (conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0) { return fmt.Errorf("Either repository_name or repository_id must be set for %s target", target) } return nil diff --git a/github/respository_rules_utils.go b/github/respository_rules_utils.go index d90ef5b868..6730f6ee68 100644 --- a/github/respository_rules_utils.go +++ b/github/respository_rules_utils.go @@ -570,7 +570,7 @@ func flattenRules(rules *github.RepositoryRulesetRules, org bool) []any { "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) + log.Printf("[DEBUG] Flattened Pull Request rules slice: %#v", pullRequestSlice) rulesMap["pull_request"] = pullRequestSlice } From 519ea54cdb3035806fb28caa2ab6fd0002209b0f Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 02:14:07 +0200 Subject: [PATCH 46/72] Rename test resources for easier debugging Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 0bb29e44cb..3650aefbcb 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -690,8 +690,9 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) t.Run("Validates push target rejects ref_name", func(t *testing.T) { + resourceName := "test-push-reject-ref-name" config := fmt.Sprintf(` - resource "github_organization_ruleset" "test" { + resource "github_organization_ruleset" "%s" { name = "test-push-with-ref-%s" target = "push" enforcement = "active" @@ -711,7 +712,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { creation = true } } - `, randomID) + `, resourceName, randomID) testCase := func(t *testing.T, mode string) { resource.Test(t, resource.TestCase{ @@ -736,8 +737,9 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) t.Run("Validates repository target rejects ref_name", func(t *testing.T) { + resourceName := "test-repository-reject-ref-name" config := fmt.Sprintf(` - resource "github_organization_ruleset" "test" { + resource "github_organization_ruleset" "%s" { name = "test-repo-with-ref-%s" target = "repository" enforcement = "active" @@ -757,7 +759,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { creation = true } } - `, randomID) + `, resourceName, randomID) testCase := func(t *testing.T, mode string) { resource.Test(t, resource.TestCase{ From 21e2e02bb45884b40f529e69d4d661889a317fcb Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 02:26:23 +0200 Subject: [PATCH 47/72] Fix linter issues Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 415d55543c..1cf1f47442 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -735,7 +735,7 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso return []*schema.ResourceData{d}, nil } -func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, meta any) error { +func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, _ any) error { target := d.Get("target").(string) conditions := d.Get("conditions").([]any)[0].(map[string]any) tflog.Debug(ctx, "Validating conditions field for branch and tag targets", map[string]any{"target": target, "conditions": conditions}) @@ -743,12 +743,12 @@ func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schem return fmt.Errorf("ref_name must be set for %s target", target) } if (conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0) && (conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0) { - return fmt.Errorf("Either repository_name or repository_id must be set for %s target", target) + return fmt.Errorf("either repository_name or repository_id must be set for %s target", target) } return nil } -func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { +func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.ResourceDiff, _ any) error { target := d.Get("target").(string) conditions := d.Get("conditions").([]any)[0].(map[string]any) tflog.Debug(ctx, "Validating conditions field for push target", map[string]any{"target": target, "conditions": conditions}) @@ -759,7 +759,7 @@ func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.Resourc return nil } -func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { +func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.ResourceDiff, _ any) error { target := d.Get("target").(string) conditions := d.Get("conditions").([]any)[0].(map[string]any) tflog.Debug(ctx, "Validating conditions field for repository target", map[string]any{"target": target, "conditions": conditions}) @@ -778,7 +778,7 @@ func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.Resourc tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target}) conditionsRaw := d.Get("conditions").([]any) - if conditionsRaw == nil || len(conditionsRaw) == 0 { + if len(conditionsRaw) == 0 { tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]any{"target": target}) return nil } From 77e8cfed2f65ee3b009bef456900a9a335480587 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 8 Dec 2025 15:40:59 +0200 Subject: [PATCH 48/72] Add validation to ensure `rules.required_status_checks.required_checks.context` is not empty Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 7 +-- ...source_github_organization_ruleset_test.go | 50 +++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 1cf1f47442..96868f7f89 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -260,9 +260,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "context": { - Type: schema.TypeString, - Required: true, - Description: "The status check context name that must be present on the commit.", + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringIsNotEmpty), + Description: "The status check context name that must be present on the commit.", }, "integration_id": { Type: schema.TypeInt, diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 3650aefbcb..a6d4310b21 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -735,6 +735,56 @@ func TestGithubOrganizationRulesets(t *testing.T) { t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") }) }) + t.Run("Validates required_status_checks context is not empty", func(t *testing.T) { + resourceName := "test-required-status-checks-context-is-not-empty" + config := fmt.Sprintf(` + resource "github_organization_ruleset" "%s" { + name = "test-context-is-not-empty-%s" + target = "branch" + enforcement = "active" + + conditions { + ref_name { + include = ["~ALL"] + exclude = [] + } + repository_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + required_status_checks { + required_check { + context = "" + } + } + } + } + `, resourceName, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("expected \"context\" to not be an empty string"), + }, + }, + }) + } + + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) + }) + + t.Run("with an organization account", func(t *testing.T) { + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") + }) + }) t.Run("Validates repository target rejects ref_name", func(t *testing.T) { resourceName := "test-repository-reject-ref-name" From 6782aabec9b4a7e2397d9e56b578114ab850ae12 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 8 Dec 2025 15:42:16 +0200 Subject: [PATCH 49/72] `go mod tidy` Signed-off-by: Timo Sand --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index eacd1663f7..eca003b564 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/google/go-github/v77 v77.0.0 github.com/google/uuid v1.6.0 github.com/hashicorp/go-cty v1.5.0 + github.com/hashicorp/terraform-plugin-log v0.9.0 github.com/hashicorp/terraform-plugin-sdk/v2 v2.38.1 github.com/shurcooL/githubv4 v0.0.0-20221126192849-0b5c4c7994eb github.com/stretchr/testify v1.11.1 @@ -129,7 +130,6 @@ require ( github.com/hashicorp/terraform-exec v0.23.1 // indirect github.com/hashicorp/terraform-json v0.27.1 // indirect github.com/hashicorp/terraform-plugin-go v0.29.0 // indirect - github.com/hashicorp/terraform-plugin-log v0.9.0 // indirect github.com/hashicorp/terraform-registry-address v0.4.0 // indirect github.com/hashicorp/terraform-svchost v0.1.1 // indirect github.com/hashicorp/yamux v0.1.2 // indirect From 97e235105dcd2dafd1606451729325de56a694c9 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 8 Dec 2025 15:59:19 +0200 Subject: [PATCH 50/72] Add test to ensure that `required_checks` is always required Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 85 +++++++++++++++---- 1 file changed, 68 insertions(+), 17 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index a6d4310b21..65973c93d9 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -735,9 +735,11 @@ func TestGithubOrganizationRulesets(t *testing.T) { t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") }) }) - t.Run("Validates required_status_checks context is not empty", func(t *testing.T) { - resourceName := "test-required-status-checks-context-is-not-empty" - config := fmt.Sprintf(` + + t.Run("Validates rules.required_status_checks", func(t *testing.T) { + t.Run("required_check.context should not be empty", func(t *testing.T) { + resourceName := "test-required-status-checks-context-is-not-empty" + config := fmt.Sprintf(` resource "github_organization_ruleset" "%s" { name = "test-context-is-not-empty-%s" target = "branch" @@ -764,25 +766,74 @@ func TestGithubOrganizationRulesets(t *testing.T) { } `, resourceName, randomID) - testCase := func(t *testing.T, mode string) { - resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessMode(t, mode) }, - Providers: testAccProviders, - Steps: []resource.TestStep{ - { - Config: config, - ExpectError: regexp.MustCompile("expected \"context\" to not be an empty string"), + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("expected \"context\" to not be an empty string"), + }, }, - }, + }) + } + + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) }) - } - t.Run("with an enterprise account", func(t *testing.T) { - testCase(t, enterprise) + t.Run("with an organization account", func(t *testing.T) { + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") + }) }) + t.Run("required_check should be required when strict_required_status_checks_policy is set", func(t *testing.T) { + resourceName := "test-required-check-is-required" + config := fmt.Sprintf(` + resource "github_organization_ruleset" "%s" { + name = "test-required-with-%s" + target = "branch" + enforcement = "active" - t.Run("with an organization account", func(t *testing.T) { - t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") + conditions { + ref_name { + include = ["~ALL"] + exclude = [] + } + repository_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + required_status_checks { + strict_required_status_checks_policy = true + } + } + } + `, resourceName, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("Insufficient required_check blocks"), + }, + }, + }) + } + + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) + }) + + t.Run("with an organization account", func(t *testing.T) { + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") + }) }) }) From 87173612d36cc21552c8107591e4ea347d7af854 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 10 Dec 2025 21:19:00 +0200 Subject: [PATCH 51/72] Remove `repository` as possible `target` value. It's not available in this SDK version Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 22 ++------- ...source_github_organization_ruleset_test.go | 47 ------------------- 2 files changed, 3 insertions(+), 66 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 96868f7f89..f6e4624aca 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -40,8 +40,8 @@ func resourceGithubOrganizationRuleset() *schema.Resource { "target": { Type: schema.TypeString, Required: true, - ValidateFunc: validation.StringInSlice([]string{"branch", "tag", "push", "repository"}, false), - Description: "The target of the ruleset. Possible values are `branch`, `tag`, `push` and `repository`.", + ValidateFunc: validation.StringInSlice([]string{"branch", "tag", "push"}, false), + Description: "The target of the ruleset. Possible values are `branch`, `tag`, and `push`.", }, "enforcement": { Type: schema.TypeString, @@ -91,7 +91,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Type: schema.TypeList, Optional: true, MaxItems: 1, - Description: "Parameters for an organization ruleset condition. For `branch` and `tag` targets, `ref_name` is required alongside one of `repository_name` or `repository_id`. The `push` target does not require `ref_name` conditions. For `repository` target, the conditions object should only contain the `repository_name` and the `repository_id`.", + Description: "Parameters for an organization ruleset condition. For `branch` and `tag` targets, `ref_name` is required alongside one of `repository_name` or `repository_id`. The `push` target does not require `ref_name` conditions.", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "ref_name": { @@ -760,20 +760,6 @@ func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.Resourc return nil } -func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.ResourceDiff, _ any) error { - target := d.Get("target").(string) - conditions := d.Get("conditions").([]any)[0].(map[string]any) - tflog.Debug(ctx, "Validating conditions field for repository target", map[string]any{"target": target, "conditions": conditions}) - - if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { - return fmt.Errorf("ref_name must not be set for %s target", target) - } - if conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0 || conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0 { - return fmt.Errorf("repository_name or repository_id must be set for %s target", target) - } - return nil -} - func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { target := d.Get("target").(string) tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target}) @@ -789,8 +775,6 @@ func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.Resourc return validateConditionsFieldForBranchAndTagTargets(ctx, d, meta) case "push": return validateConditionsFieldForPushTarget(ctx, d, meta) - case "repository": - return validateConditionsFieldForRepositoryTarget(ctx, d, meta) } return nil } diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 65973c93d9..ff5d485ab1 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -836,53 +836,6 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) }) }) - - t.Run("Validates repository target rejects ref_name", func(t *testing.T) { - resourceName := "test-repository-reject-ref-name" - config := fmt.Sprintf(` - resource "github_organization_ruleset" "%s" { - name = "test-repo-with-ref-%s" - target = "repository" - enforcement = "active" - - conditions { - ref_name { - include = ["~ALL"] - exclude = [] - } - repository_name { - include = ["~ALL"] - exclude = [] - } - } - - rules { - creation = true - } - } - `, resourceName, randomID) - - testCase := func(t *testing.T, mode string) { - resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessMode(t, mode) }, - Providers: testAccProviders, - Steps: []resource.TestStep{ - { - Config: config, - ExpectError: regexp.MustCompile("ref_name must not be set for repository target"), - }, - }, - }) - } - - t.Run("with an enterprise account", func(t *testing.T) { - testCase(t, enterprise) - }) - - t.Run("with an organization account", func(t *testing.T) { - t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") - }) - }) } func TestOrganizationPushRulesetSupport(t *testing.T) { From e8b41f4dd644add8e6154f1ecee7ebe4e7bc78fb Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 10 Dec 2025 21:54:37 +0200 Subject: [PATCH 52/72] Improve legibility of `conditions` description 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 f6e4624aca..1db289be6a 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -91,7 +91,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Type: schema.TypeList, Optional: true, MaxItems: 1, - Description: "Parameters for an organization ruleset condition. For `branch` and `tag` targets, `ref_name` is required alongside one of `repository_name` or `repository_id`. The `push` target does not require `ref_name` conditions.", + Description: "Parameters for an organization ruleset condition. `ref_name` is required for `branch` and `tag` targets, but must not be set for `push` targets. One of `repository_name` or `repository_id` is always required.", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "ref_name": { From 66e0a1d7b5f096b1e6d9e0ecb34e569c33fdcd38 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 14 Dec 2025 09:07:47 +0200 Subject: [PATCH 53/72] Update descriptions Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 1db289be6a..8f973a6058 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -47,7 +47,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Type: schema.TypeString, Required: true, ValidateFunc: validation.StringInSlice([]string{"disabled", "active", "evaluate"}, false), - Description: "Possible values for Enforcement are `disabled`, `active`, `evaluate`. Note: `evaluate` is currently only supported for owners of type `organization`.", + Description: "The enforcement level of the ruleset. `evaluate` allows admins to test rules before enforcing them. Possible values are `disabled`, `active`, and `evaluate`. Note: `evaluate` is only available for Enterprise plans.", }, "bypass_actors": { 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 @@ -66,7 +66,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Type: schema.TypeString, Required: true, ValidateFunc: validation.StringInSlice([]string{"Integration", "OrganizationAdmin", "RepositoryRole", "Team", "DeployKey"}, false), - Description: "The type of actor that can bypass a ruleset. See https://docs.github.com/en/rest/orgs/rules for more information", + Description: "The type of actor that can bypass a ruleset. Can be one of: `Integration`, `OrganizationAdmin`, `RepositoryRole`, `Team`, or `DeployKey`.", }, "bypass_mode": { Type: schema.TypeString, @@ -95,9 +95,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "ref_name": { - Type: schema.TypeList, - Optional: true, - MaxItems: 1, + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Description: "Targets refs that match the specified patterns. Required for `branch` and `tag` targets.", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "include": { @@ -123,6 +124,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Type: schema.TypeList, Optional: true, MaxItems: 1, + Description: "Targets repositories that match the specified name patterns.", ExactlyOneOf: []string{"conditions.0.repository_id"}, AtLeastOneOf: []string{"conditions.0.repository_id"}, Elem: &schema.Resource{ @@ -291,7 +293,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { "non_fast_forward": { Type: schema.TypeBool, Optional: true, - Description: "Prevent users with push access from force pushing to branches.", + Description: "Prevent users with push access from force pushing to refs.", }, "commit_message_pattern": { Type: schema.TypeList, @@ -595,8 +597,9 @@ func resourceGithubOrganizationRuleset() *schema.Resource { }, }, "etag": { - Type: schema.TypeString, - Computed: true, + Type: schema.TypeString, + Computed: true, + Description: "An etag representing the ruleset for caching purposes.", }, }, } From 75e58b206112180f9b60592d96ecd342065cf0aa Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 14 Dec 2025 19:50:33 +0200 Subject: [PATCH 54/72] Add Acc test for push ruleset. This turns out to be failing as there is a bug in our implementation! Unit tests and fix coming up Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 75 ++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index ff5d485ab1..a631f04212 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -709,7 +709,10 @@ func TestGithubOrganizationRulesets(t *testing.T) { } rules { - creation = true + # Push rulesets only support push-specific rules + max_file_size { + max_file_size = 100 + } } } `, resourceName, randomID) @@ -736,6 +739,76 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) }) + t.Run("Creates push ruleset with repository_name only", func(t *testing.T) { + resourceName := "test-push-repo-name-only" + config := fmt.Sprintf(` + resource "github_organization_ruleset" "%s" { + name = "test-push-%s" + target = "push" + enforcement = "active" + + conditions { + repository_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + # Push rulesets only support push-specific rules: + # file_path_restriction, max_file_path_length, file_extension_restriction, max_file_size + max_file_size { + max_file_size = 100 + } + } + } + `, resourceName, randomID) + + check := resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + fmt.Sprintf("github_organization_ruleset.%s", resourceName), + "name", + fmt.Sprintf("test-push-%s", randomID), + ), + resource.TestCheckResourceAttr( + fmt.Sprintf("github_organization_ruleset.%s", resourceName), + "target", + "push", + ), + resource.TestCheckResourceAttr( + fmt.Sprintf("github_organization_ruleset.%s", resourceName), + "enforcement", + "active", + ), + resource.TestCheckResourceAttr( + fmt.Sprintf("github_organization_ruleset.%s", resourceName), + "rules.0.max_file_size.0.max_file_size", + "100", + ), + ) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + Check: check, + }, + }, + }) + } + + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) + }) + + t.Run("with an organization account", func(t *testing.T) { + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") + }) + }) + t.Run("Validates rules.required_status_checks", func(t *testing.T) { t.Run("required_check.context should not be empty", func(t *testing.T) { resourceName := "test-required-status-checks-context-is-not-empty" From be29a43f7a3beddab9484e06853fade7549071c1 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 14 Dec 2025 19:54:08 +0200 Subject: [PATCH 55/72] Add failing test for `flattenConditions` with no `ref_name` condition Signed-off-by: Timo Sand --- github/respository_rules_utils_test.go | 46 ++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/github/respository_rules_utils_test.go b/github/respository_rules_utils_test.go index df3b042f24..04513e2d10 100644 --- a/github/respository_rules_utils_test.go +++ b/github/respository_rules_utils_test.go @@ -418,3 +418,49 @@ func TestCompletePushRulesetSupport(t *testing.T) { t.Errorf("Expected 3 restricted file extensions, got %d", len(restrictedExts)) } } + +func TestFlattenConditions_PushRuleset_WithRepositoryNameOnly(t *testing.T) { + // Push rulesets don't use ref_name - they only have repository_name or repository_id. + // flattenConditions should return the conditions even when RefName is nil. + conditions := &github.RepositoryRulesetConditions{ + RefName: nil, // Push rulesets don't have ref_name + RepositoryName: &github.RepositoryRulesetRepositoryNamesConditionParameters{ + Include: []string{"~ALL"}, + Exclude: []string{}, + }, + } + + result := flattenConditions(conditions, true) // org=true for organization rulesets + + if len(result) != 1 { + t.Fatalf("Expected 1 conditions block, got %d", len(result)) + } + + conditionsMap := result[0].(map[string]any) + + // ref_name should be empty for push rulesets + refNameSlice, ok := conditionsMap["ref_name"].([]map[string]any) + if !ok { + t.Fatalf("Expected ref_name to be []map[string]any, got %T", conditionsMap["ref_name"]) + } + if len(refNameSlice) != 0 { + t.Errorf("Expected ref_name to be empty for push ruleset, got %d elements", len(refNameSlice)) + } + + // repository_name should be present + repoNameSlice, ok := conditionsMap["repository_name"].([]map[string]any) + if !ok { + t.Fatalf("Expected repository_name to be []map[string]any, got %T", conditionsMap["repository_name"]) + } + if len(repoNameSlice) != 1 { + t.Fatalf("Expected 1 repository_name block, got %d", len(repoNameSlice)) + } + + include, ok := repoNameSlice[0]["include"].([]string) + if !ok { + t.Fatalf("Expected include to be []string, got %T", repoNameSlice[0]["include"]) + } + if len(include) != 1 || include[0] != "~ALL" { + t.Errorf("Expected include to be [~ALL], got %v", include) + } +} From 73f817b32b6201a6ce3f0d7c246b4cbeaa3fa066 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 15 Dec 2025 06:58:20 +0200 Subject: [PATCH 56/72] Fix `flattenConditions` to work with `push` rulesets Signed-off-by: Timo Sand --- github/respository_rules_utils.go | 14 ++++++++------ github/respository_rules_utils_test.go | 9 +++------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/github/respository_rules_utils.go b/github/respository_rules_utils.go index 6730f6ee68..8384dc0bfd 100644 --- a/github/respository_rules_utils.go +++ b/github/respository_rules_utils.go @@ -225,19 +225,21 @@ func expandConditions(input []any, org bool) *github.RepositoryRulesetConditions } func flattenConditions(conditions *github.RepositoryRulesetConditions, org bool) []any { - if conditions == nil || conditions.RefName == nil { + if conditions == nil { return []any{} } conditionsMap := make(map[string]any) refNameSlice := make([]map[string]any, 0) - refNameSlice = append(refNameSlice, map[string]any{ - "include": conditions.RefName.Include, - "exclude": conditions.RefName.Exclude, - }) + if conditions.RefName != nil { + refNameSlice = append(refNameSlice, map[string]any{ + "include": conditions.RefName.Include, + "exclude": conditions.RefName.Exclude, + }) - conditionsMap["ref_name"] = refNameSlice + conditionsMap["ref_name"] = refNameSlice + } // org-only fields if org { diff --git a/github/respository_rules_utils_test.go b/github/respository_rules_utils_test.go index 04513e2d10..6261b5c27f 100644 --- a/github/respository_rules_utils_test.go +++ b/github/respository_rules_utils_test.go @@ -439,12 +439,9 @@ func TestFlattenConditions_PushRuleset_WithRepositoryNameOnly(t *testing.T) { conditionsMap := result[0].(map[string]any) // ref_name should be empty for push rulesets - refNameSlice, ok := conditionsMap["ref_name"].([]map[string]any) - if !ok { - t.Fatalf("Expected ref_name to be []map[string]any, got %T", conditionsMap["ref_name"]) - } - if len(refNameSlice) != 0 { - t.Errorf("Expected ref_name to be empty for push ruleset, got %d elements", len(refNameSlice)) + refNameSlice := conditionsMap["ref_name"] + if refNameSlice != nil { + t.Fatalf("Expected ref_name to be nil, got %T", conditionsMap["ref_name"]) } // repository_name should be present From b46cae41f464ec0b6fa66951a352b2b8747fa4a9 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 15 Dec 2025 07:01:56 +0200 Subject: [PATCH 57/72] Add more tests for `flattenConditions` Signed-off-by: Timo Sand --- github/respository_rules_utils_test.go | 109 +++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/github/respository_rules_utils_test.go b/github/respository_rules_utils_test.go index 6261b5c27f..164380b600 100644 --- a/github/respository_rules_utils_test.go +++ b/github/respository_rules_utils_test.go @@ -461,3 +461,112 @@ func TestFlattenConditions_PushRuleset_WithRepositoryNameOnly(t *testing.T) { t.Errorf("Expected include to be [~ALL], got %v", include) } } + +func TestFlattenConditions_BranchRuleset_WithRefNameAndRepositoryName(t *testing.T) { + // Branch/tag rulesets have both ref_name and repository_name. + // This test ensures we didn't break the existing behavior. + conditions := &github.RepositoryRulesetConditions{ + RefName: &github.RepositoryRulesetRefConditionParameters{ + Include: []string{"~DEFAULT_BRANCH", "refs/heads/main"}, + Exclude: []string{"refs/heads/experimental-*"}, + }, + RepositoryName: &github.RepositoryRulesetRepositoryNamesConditionParameters{ + Include: []string{"~ALL"}, + Exclude: []string{"test-*"}, + }, + } + + result := flattenConditions(conditions, true) // org=true for organization rulesets + + if len(result) != 1 { + t.Fatalf("Expected 1 conditions block, got %d", len(result)) + } + + conditionsMap := result[0].(map[string]any) + + // ref_name should be present for branch/tag rulesets + refNameSlice, ok := conditionsMap["ref_name"].([]map[string]any) + if !ok { + t.Fatalf("Expected ref_name to be []map[string]any, got %T", conditionsMap["ref_name"]) + } + if len(refNameSlice) != 1 { + t.Fatalf("Expected 1 ref_name block, got %d", len(refNameSlice)) + } + + refInclude, ok := refNameSlice[0]["include"].([]string) + if !ok { + t.Fatalf("Expected ref_name include to be []string, got %T", refNameSlice[0]["include"]) + } + if len(refInclude) != 2 { + t.Errorf("Expected 2 ref_name includes, got %d", len(refInclude)) + } + + refExclude, ok := refNameSlice[0]["exclude"].([]string) + if !ok { + t.Fatalf("Expected ref_name exclude to be []string, got %T", refNameSlice[0]["exclude"]) + } + if len(refExclude) != 1 { + t.Errorf("Expected 1 ref_name exclude, got %d", len(refExclude)) + } + + // repository_name should also be present + repoNameSlice, ok := conditionsMap["repository_name"].([]map[string]any) + if !ok { + t.Fatalf("Expected repository_name to be []map[string]any, got %T", conditionsMap["repository_name"]) + } + if len(repoNameSlice) != 1 { + t.Fatalf("Expected 1 repository_name block, got %d", len(repoNameSlice)) + } + + repoInclude, ok := repoNameSlice[0]["include"].([]string) + if !ok { + t.Fatalf("Expected repository_name include to be []string, got %T", repoNameSlice[0]["include"]) + } + if len(repoInclude) != 1 || repoInclude[0] != "~ALL" { + t.Errorf("Expected repository_name include to be [~ALL], got %v", repoInclude) + } + + repoExclude, ok := repoNameSlice[0]["exclude"].([]string) + if !ok { + t.Fatalf("Expected repository_name exclude to be []string, got %T", repoNameSlice[0]["exclude"]) + } + if len(repoExclude) != 1 || repoExclude[0] != "test-*" { + t.Errorf("Expected repository_name exclude to be [test-*], got %v", repoExclude) + } +} + +func TestFlattenConditions_PushRuleset_WithRepositoryIdOnly(t *testing.T) { + // Push rulesets can also use repository_id instead of repository_name. + conditions := &github.RepositoryRulesetConditions{ + RefName: nil, // Push rulesets don't have ref_name + RepositoryID: &github.RepositoryRulesetRepositoryIDsConditionParameters{ + RepositoryIDs: []int64{12345, 67890}, + }, + } + + result := flattenConditions(conditions, true) // org=true for organization rulesets + + if len(result) != 1 { + t.Fatalf("Expected 1 conditions block, got %d", len(result)) + } + + conditionsMap := result[0].(map[string]any) + + // ref_name should be nil for push rulesets + refNameSlice := conditionsMap["ref_name"] + if refNameSlice != nil { + t.Fatalf("Expected ref_name to be nil, got %T", conditionsMap["ref_name"]) + } + + // repository_id should be present + repoIDs, ok := conditionsMap["repository_id"].([]int64) + if !ok { + t.Fatalf("Expected repository_id to be []int64, got %T", conditionsMap["repository_id"]) + } + if len(repoIDs) != 2 { + t.Fatalf("Expected 2 repository IDs, got %d", len(repoIDs)) + } + if repoIDs[0] != 12345 || repoIDs[1] != 67890 { + t.Errorf("Expected repository IDs [12345, 67890], got %v", repoIDs) + } +} From 6b91f57f4b2ee155831d5bdd81d5a615ba20e2b5 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 15 Dec 2025 21:10:46 +0200 Subject: [PATCH 58/72] Enable debug logging in `flattenConditions` Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 2 +- github/respository_rules_utils.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 8f973a6058..e15bb0cbc7 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -670,7 +670,7 @@ func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.Resour _ = d.Set("target", ruleset.GetTarget()) _ = d.Set("enforcement", ruleset.Enforcement) _ = d.Set("bypass_actors", flattenBypassActors(ruleset.BypassActors)) - _ = d.Set("conditions", flattenConditions(ruleset.GetConditions(), true)) + _ = d.Set("conditions", flattenConditionsWithContext(ctx, ruleset.GetConditions(), true)) _ = d.Set("rules", flattenRules(ruleset.Rules, true)) _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("ruleset_id", ruleset.ID) diff --git a/github/respository_rules_utils.go b/github/respository_rules_utils.go index 8384dc0bfd..7645a92ecf 100644 --- a/github/respository_rules_utils.go +++ b/github/respository_rules_utils.go @@ -1,11 +1,13 @@ package github import ( + "context" "log" "reflect" "sort" "github.com/google/go-github/v77/github" + "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -225,7 +227,12 @@ func expandConditions(input []any, org bool) *github.RepositoryRulesetConditions } func flattenConditions(conditions *github.RepositoryRulesetConditions, org bool) []any { + return flattenConditionsWithContext(context.TODO(), conditions, org) +} + +func flattenConditionsWithContext(ctx context.Context, conditions *github.RepositoryRulesetConditions, org bool) []any { if conditions == nil { + tflog.Debug(ctx, "Conditions are nil, returning empty list", map[string]any{"conditions": conditions}) return []any{} } From cec204f1aedd5d7e3ddc6773c51dbe980afe8686 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 15 Dec 2025 21:18:22 +0200 Subject: [PATCH 59/72] Ensures that `flattenConditions` returns an empty list on empty API response Signed-off-by: Timo Sand --- github/respository_rules_utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/respository_rules_utils.go b/github/respository_rules_utils.go index 7645a92ecf..57ffaea8a9 100644 --- a/github/respository_rules_utils.go +++ b/github/respository_rules_utils.go @@ -231,8 +231,8 @@ func flattenConditions(conditions *github.RepositoryRulesetConditions, org bool) } func flattenConditionsWithContext(ctx context.Context, conditions *github.RepositoryRulesetConditions, org bool) []any { - if conditions == nil { - tflog.Debug(ctx, "Conditions are nil, returning empty list", map[string]any{"conditions": conditions}) + if conditions == nil || reflect.DeepEqual(conditions, &github.RepositoryRulesetConditions{}) { + tflog.Debug(ctx, "Conditions are empty, returning empty list") return []any{} } From 18c860ee976ae5d8cd34327569af8b64bedd9ebe Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 16 Dec 2025 19:35:59 +0200 Subject: [PATCH 60/72] Add validation for `push` `rules` As they differ from `branch` and `tag` rules Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 127 +++++++++++++++++- ...source_github_organization_ruleset_test.go | 94 +++++++++++++ 2 files changed, 220 insertions(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index e15bb0cbc7..e6884b295f 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -12,6 +12,7 @@ import ( "github.com/google/go-github/v77/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" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) @@ -28,7 +29,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { SchemaVersion: 1, - CustomizeDiff: validateConditionsFieldBasedOnTarget, + CustomizeDiff: customdiff.All( + validateConditionsFieldBasedOnTarget, + validateRulesFieldBasedOnTarget, + ), Schema: map[string]*schema.Schema{ "name": { @@ -781,3 +785,124 @@ func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.Resourc } return nil } + +// branchTagOnlyRules contains rules that are only valid for branch and tag targets. +// +// These rules apply to ref-based operations (branches and tags) and are not supported +// for push rulesets which operate on file content. +// +// To verify/maintain this list: +// 1. Check the GitHub API documentation for organization rulesets: +// https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-ruleset +// 2. The API docs don't clearly separate push vs branch/tag rules. To verify, +// attempt to create a push ruleset via API or UI with each rule type. +// Push rulesets will reject branch/tag rules with "Invalid rule ''" error. +// 3. Generally, push rules deal with file content (paths, sizes, extensions), +// while branch/tag rules deal with ref lifecycle and merge requirements. +var branchTagOnlyRules = []string{ + "creation", + "update", + "deletion", + "required_linear_history", + "required_signatures", + "pull_request", + "required_status_checks", + "non_fast_forward", + "commit_message_pattern", + "commit_author_email_pattern", + "committer_email_pattern", + "branch_name_pattern", + "tag_name_pattern", + "required_workflows", + "required_code_scanning", +} + +// pushOnlyRules contains rules that are only valid for push targets. +// +// These rules apply to push operations and control what content can be pushed +// to repositories. They are not supported for branch or tag rulesets. +// +// To verify/maintain this list: +// 1. Check the GitHub API documentation for organization rulesets: +// https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-ruleset +// 2. The API docs don't clearly separate push vs branch/tag rules. To verify, +// attempt to create a branch ruleset via API or UI with each rule type. +// Branch rulesets will reject push-only rules with an error. +// 3. Push rules control file content: paths, sizes, extensions, path lengths. +var pushOnlyRules = []string{ + "file_path_restriction", + "max_file_path_length", + "file_extension_restriction", + "max_file_size", +} + +func validateRulesForPushTarget(ctx context.Context, d *schema.ResourceDiff, _ any) error { + tflog.Debug(ctx, "Validating rules for push target") + rulesRaw := d.Get("rules").([]any) + if len(rulesRaw) == 0 { + tflog.Debug(ctx, "No rules block, skipping validation") + return nil + } + + rules := rulesRaw[0].(map[string]any) + + for _, ruleName := range branchTagOnlyRules { + ruleValue := rules[ruleName] + if ruleValue == nil { + continue + } + switch v := ruleValue.(type) { + case bool: + if v { + tflog.Debug(ctx, "Invalid rule for push target", map[string]any{"rule": ruleName, "value": v}) + return fmt.Errorf("rule %q is not valid for push target; push targets only support: %v", ruleName, pushOnlyRules) + } + case []any: + if len(v) > 0 { + tflog.Debug(ctx, "Invalid rule for push target", map[string]any{"rule": ruleName, "value": v}) + return fmt.Errorf("rule %q is not valid for push target; push targets only support: %v", ruleName, pushOnlyRules) + } + } + } + tflog.Debug(ctx, "Rules validation passed for push target") + return nil +} + +func validateRulesForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, _ any) error { + target := d.Get("target").(string) + tflog.Debug(ctx, "Validating rules for branch/tag target", map[string]any{"target": target}) + rulesRaw := d.Get("rules").([]any) + if len(rulesRaw) == 0 { + tflog.Debug(ctx, "No rules block, skipping validation") + return nil + } + + rules := rulesRaw[0].(map[string]any) + + for _, ruleName := range pushOnlyRules { + ruleValue := rules[ruleName] + if ruleValue == nil { + continue + } + if ruleList, ok := ruleValue.([]any); ok && len(ruleList) > 0 { + tflog.Debug(ctx, "Invalid rule for branch/tag target", map[string]any{"rule": ruleName, "target": target}) + return fmt.Errorf("rule %q is only valid for push target, not for %s target", ruleName, target) + } + } + tflog.Debug(ctx, "Rules validation passed for branch/tag target", map[string]any{"target": target}) + return nil +} + +func validateRulesFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { + target := d.Get("target").(string) + tflog.Debug(ctx, "Validating rules field based on target", map[string]any{"target": target}) + + switch target { + case "branch", "tag": + return validateRulesForBranchAndTagTargets(ctx, d, meta) + case "push": + return validateRulesForPushTarget(ctx, d, meta) + } + + return nil +} diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index a631f04212..6af31e8072 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -739,6 +739,100 @@ func TestGithubOrganizationRulesets(t *testing.T) { }) }) + t.Run("Validates push target rejects branch/tag rules", func(t *testing.T) { + resourceName := "test-push-reject-branch-rules" + config := fmt.Sprintf(` + resource "github_organization_ruleset" "%s" { + name = "test-push-branch-rule-%s" + target = "push" + enforcement = "active" + + conditions { + repository_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + # 'creation' is a branch/tag rule, not valid for push target + creation = true + } + } + `, resourceName, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("rule .* is not valid for push target"), + }, + }, + }) + } + + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) + }) + + t.Run("with an organization account", func(t *testing.T) { + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") + }) + }) + + t.Run("Validates branch target rejects push-only rules", func(t *testing.T) { + resourceName := "test-branch-reject-push-rules" + config := fmt.Sprintf(` + resource "github_organization_ruleset" "%s" { + name = "test-branch-push-rule-%s" + target = "branch" + enforcement = "active" + + conditions { + ref_name { + include = ["~ALL"] + exclude = [] + } + repository_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + # 'max_file_size' is a push-only rule, not valid for branch target + max_file_size { + max_file_size = 100 + } + } + } + `, resourceName, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("rule .* is only valid for push target"), + }, + }, + }) + } + + t.Run("with an enterprise account", func(t *testing.T) { + testCase(t, enterprise) + }) + + t.Run("with an organization account", func(t *testing.T) { + t.Skip("organization account not supported for this operation, since it needs a paid Team plan.") + }) + }) + t.Run("Creates push ruleset with repository_name only", func(t *testing.T) { resourceName := "test-push-repo-name-only" config := fmt.Sprintf(` From b3ca429d9ef6da198c2f2ed34c86d48323b621b2 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 16 Dec 2025 21:05:53 +0200 Subject: [PATCH 61/72] `github_repository`: Remove comment which doesn't hold true anymore Signed-off-by: Timo Sand --- github/resource_github_repository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index b0791f1217..d06b95e08b 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -885,7 +885,7 @@ func resourceGithubRepositoryRead(ctx context.Context, d *schema.ResourceData, m func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { // 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) + // archive the repository if d.Get("archived").(bool) && !d.HasChange("archived") { log.Printf("[INFO] Skipping update of archived repository") return nil From bedfb3498ad4e2e788b7f5bf599eba853382c55d Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 16 Dec 2025 21:08:52 +0200 Subject: [PATCH 62/72] `github_repository`: Ensures that we don't try to PATCH an archived repo Signed-off-by: Timo Sand --- github/resource_github_repository.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index d06b95e08b..88c3725f28 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -895,7 +895,7 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, repoReq := resourceGithubRepositoryObject(d) - // handle visibility updates separately from other fields + // handle visibility updates separately from other fields // TODO: Review if this behaviour is still needed repoReq.Visibility = nil if !d.HasChange("security_and_analysis") { @@ -983,7 +983,7 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, } } - if d.HasChange("visibility") { + if d.HasChange("visibility") && !d.Get("archived").(bool) { o, n := d.GetChange("visibility") repoReq.Visibility = github.Ptr(n.(string)) log.Printf("[DEBUG] Updating repository visibility from %s to %s", o, n) @@ -997,7 +997,7 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, log.Printf("[DEBUG] No visibility update required. visibility: %s", d.Get("visibility")) } - if d.HasChange("private") { + if d.HasChange("private") && !d.Get("archived").(bool) { o, n := d.GetChange("private") repoReq.Private = github.Ptr(n.(bool)) log.Printf("[DEBUG] Updating repository privacy from %v to %v", o, n) From 2e01393356b643a25fd3349e3ee30b466a235f47 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 16 Dec 2025 21:16:10 +0200 Subject: [PATCH 63/72] `github_repository_ruleset`: Change `TestGithubRepositoryRulesetArchived` repos to be private This allows even EMU users to run these tests Signed-off-by: Timo Sand --- github/resource_github_repository_ruleset_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/github/resource_github_repository_ruleset_test.go b/github/resource_github_repository_ruleset_test.go index ee5d3c1c28..025e9d9ac4 100644 --- a/github/resource_github_repository_ruleset_test.go +++ b/github/resource_github_repository_ruleset_test.go @@ -1095,6 +1095,8 @@ func TestGithubRepositoryRulesetArchived(t *testing.T) { name = "tf-acc-test-archive-%s" auto_init = true archived = false + + visibility = "private" # Enables test even in GHEC EMU } resource "github_repository_ruleset" "test" { name = "test" @@ -1122,6 +1124,8 @@ func TestGithubRepositoryRulesetArchived(t *testing.T) { name = "tf-acc-test-archive-create-%s" auto_init = true archived = true + + visibility = "private" # Enables test even in GHEC EMU } resource "github_repository_ruleset" "test" { name = "test" From 760eee315f0298614168151a482978960ac4047d Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 16 Dec 2025 21:17:08 +0200 Subject: [PATCH 64/72] Rename test functions to include `Acc` prefix to conform to TF best practices Signed-off-by: Timo Sand --- github/resource_github_repository_ruleset_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/resource_github_repository_ruleset_test.go b/github/resource_github_repository_ruleset_test.go index 025e9d9ac4..7d0a539308 100644 --- a/github/resource_github_repository_ruleset_test.go +++ b/github/resource_github_repository_ruleset_test.go @@ -1086,7 +1086,7 @@ func TestGithubRepositoryRulesets(t *testing.T) { }) } -func TestGithubRepositoryRulesetArchived(t *testing.T) { +func TestAccGithubRepositoryRulesetArchived(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) t.Run("skips update and delete on archived repository", func(t *testing.T) { From bff5892665e6fe60b67bb86c54f991859a59eb8f Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 16 Dec 2025 22:21:21 +0200 Subject: [PATCH 65/72] `github_team`: Update to use context-aware provider functions This makes debugging easier as we get DEBUG logs from HTTP req/res Signed-off-by: Timo Sand --- github/resource_github_team.go | 92 +++++++++++++++++----------------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/github/resource_github_team.go b/github/resource_github_team.go index 70c7117a32..6a28fde089 100644 --- a/github/resource_github_team.go +++ b/github/resource_github_team.go @@ -8,6 +8,7 @@ import ( "strconv" "github.com/google/go-github/v77/github" + "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" "github.com/shurcooL/githubv4" @@ -15,12 +16,12 @@ import ( func resourceGithubTeam() *schema.Resource { return &schema.Resource{ - Create: resourceGithubTeamCreate, - Read: resourceGithubTeamRead, - Update: resourceGithubTeamUpdate, - Delete: resourceGithubTeamDelete, + CreateContext: resourceGithubTeamCreate, + ReadContext: resourceGithubTeamRead, + UpdateContext: resourceGithubTeamUpdate, + DeleteContext: resourceGithubTeamDelete, Importer: &schema.ResourceImporter{ - State: resourceGithubTeamImport, + StateContext: resourceGithubTeamImport, }, CustomizeDiff: customdiff.Sequence( @@ -104,10 +105,10 @@ func resourceGithubTeam() *schema.Resource { } } -func resourceGithubTeamCreate(d *schema.ResourceData, meta any) error { +func resourceGithubTeamCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { err := checkOrganization(meta) if err != nil { - return err + return diag.FromErr(err) } client := meta.(*Owner).v3client @@ -128,16 +129,15 @@ func resourceGithubTeamCreate(d *schema.ResourceData, meta any) error { if parentTeamID, ok := d.GetOk("parent_team_id"); ok { teamID, err := getTeamID(parentTeamID.(string), meta) if err != nil { - return err + return diag.FromErr(err) } newTeam.ParentTeamID = &teamID } - ctx := context.Background() githubTeam, _, err := client.Teams.CreateTeam(ctx, ownerName, newTeam) if err != nil { - return err + return diag.FromErr(err) } /* @@ -161,7 +161,7 @@ func resourceGithubTeamCreate(d *schema.ResourceData, meta any) error { newTeam, false) if err != nil { - return err + return diag.FromErr(err) } } @@ -169,18 +169,18 @@ func resourceGithubTeamCreate(d *schema.ResourceData, meta any) error { if !create_default_maintainer { log.Printf("[DEBUG] Removing default maintainer from team: %s (%s)", name, ownerName) if err := removeDefaultMaintainer(*githubTeam.Slug, meta); err != nil { - return err + return diag.FromErr(err) } } d.SetId(strconv.FormatInt(githubTeam.GetID(), 10)) - return resourceGithubTeamRead(d, meta) + return resourceGithubTeamRead(ctx, d, meta) } -func resourceGithubTeamRead(d *schema.ResourceData, meta any) error { +func resourceGithubTeamRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { err := checkOrganization(meta) if err != nil { - return err + return diag.FromErr(err) } client := meta.(*Owner).v3client @@ -188,9 +188,9 @@ func resourceGithubTeamRead(d *schema.ResourceData, meta any) error { id, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { - return unconvertibleIdErr(d.Id(), err) + return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } - ctx := context.WithValue(context.Background(), ctxId, d.Id()) + ctx = context.WithValue(ctx, ctxId, d.Id()) if !d.IsNewResource() { ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string)) } @@ -210,62 +210,62 @@ func resourceGithubTeamRead(d *schema.ResourceData, meta any) error { return nil } } - return err + return diag.FromErr(err) } if err = d.Set("etag", resp.Header.Get("ETag")); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("description", team.GetDescription()); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("name", team.GetName()); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("privacy", team.GetPrivacy()); err != nil { - return err + return diag.FromErr(err) } if parent := team.Parent; parent != nil { if err = d.Set("parent_team_id", strconv.FormatInt(team.Parent.GetID(), 10)); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("parent_team_read_id", strconv.FormatInt(team.Parent.GetID(), 10)); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("parent_team_read_slug", parent.Slug); err != nil { - return err + return diag.FromErr(err) } } else { if err = d.Set("parent_team_id", ""); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("parent_team_read_id", ""); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("parent_team_read_slug", ""); err != nil { - return err + return diag.FromErr(err) } } if err = d.Set("ldap_dn", team.GetLDAPDN()); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("slug", team.GetSlug()); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("node_id", team.GetNodeID()); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("members_count", team.GetMembersCount()); err != nil { - return err + return diag.FromErr(err) } return nil } -func resourceGithubTeamUpdate(d *schema.ResourceData, meta any) error { +func resourceGithubTeamUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { err := checkOrganization(meta) if err != nil { - return err + return diag.FromErr(err) } client := meta.(*Owner).v3client @@ -279,7 +279,7 @@ func resourceGithubTeamUpdate(d *schema.ResourceData, meta any) error { if parentTeamID, ok := d.GetOk("parent_team_id"); ok { teamID, err := getTeamID(parentTeamID.(string), meta) if err != nil { - return err + return diag.FromErr(err) } editedTeam.ParentTeamID = &teamID removeParentTeam = false @@ -289,15 +289,15 @@ func resourceGithubTeamUpdate(d *schema.ResourceData, meta any) error { teamID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { - return unconvertibleIdErr(d.Id(), err) + return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } - ctx := context.WithValue(context.Background(), ctxId, d.Id()) + ctx = context.WithValue(ctx, ctxId, d.Id()) //nolint:staticcheck // SA1019: EditTeamByID is deprecated but still needed for legacy compatibility team, _, err := client.Teams.EditTeamByID(ctx, meta.(*Owner).id, teamID, editedTeam, removeParentTeam) if err != nil { - return err + return diag.FromErr(err) } if d.HasChange("ldap_dn") { @@ -307,18 +307,18 @@ func resourceGithubTeamUpdate(d *schema.ResourceData, meta any) error { } _, _, err = client.Admin.UpdateTeamLDAPMapping(ctx, team.GetID(), mapping) if err != nil { - return err + return diag.FromErr(err) } } d.SetId(strconv.FormatInt(team.GetID(), 10)) - return resourceGithubTeamRead(d, meta) + return resourceGithubTeamRead(ctx, d, meta) } -func resourceGithubTeamDelete(d *schema.ResourceData, meta any) error { +func resourceGithubTeamDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { err := checkOrganization(meta) if err != nil { - return err + return diag.FromErr(err) } client := meta.(*Owner).v3client @@ -326,9 +326,9 @@ func resourceGithubTeamDelete(d *schema.ResourceData, meta any) error { id, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { - return unconvertibleIdErr(d.Id(), err) + return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } - ctx := context.WithValue(context.Background(), ctxId, d.Id()) + ctx = context.WithValue(ctx, ctxId, d.Id()) //nolint:staticcheck // SA1019: DeleteTeamByID is deprecated but still needed for legacy compatibility _, err = client.Teams.DeleteTeamByID(ctx, orgID, id) @@ -356,10 +356,10 @@ func resourceGithubTeamDelete(d *schema.ResourceData, meta any) error { } } } - return err + return diag.FromErr(err) } -func resourceGithubTeamImport(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { +func resourceGithubTeamImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { teamID, err := getTeamID(d.Id(), meta) if err != nil { return nil, err From bdef67b9a770242646a7661a0ce6cb0b14f27a3a Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 17 Dec 2025 16:26:15 +0200 Subject: [PATCH 66/72] Get `TestAccGithubRepositoryRulesets` to work Signed-off-by: Timo Sand --- ...resource_github_repository_ruleset_test.go | 117 ++++++++++++------ 1 file changed, 79 insertions(+), 38 deletions(-) diff --git a/github/resource_github_repository_ruleset_test.go b/github/resource_github_repository_ruleset_test.go index 7d0a539308..e264554b5d 100644 --- a/github/resource_github_repository_ruleset_test.go +++ b/github/resource_github_repository_ruleset_test.go @@ -12,7 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) -func TestGithubRepositoryRulesets(t *testing.T) { +func TestAccGithubRepositoryRulesets(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) t.Run("Creates and updates repository rulesets without errors", func(t *testing.T) { @@ -21,7 +21,9 @@ func TestGithubRepositoryRulesets(t *testing.T) { name = "tf-acc-test-%s" auto_init = true default_branch = "main" - vulnerability_alerts = true + vulnerability_alerts = true + + visibility = "private" # Enables test even in GHEC EMU } resource "github_repository_environment" "example" { @@ -67,6 +69,7 @@ func TestGithubRepositoryRulesets(t *testing.T) { } pull_request { + allowed_merge_methods = ["merge", "squash", "rebase"] required_approving_review_count = 2 required_review_thread_resolution = true require_code_owner_review = true @@ -85,11 +88,11 @@ func TestGithubRepositoryRulesets(t *testing.T) { } required_code_scanning { - required_code_scanning_tool { - alerts_threshold = "errors" - security_alerts_threshold = "high_or_higher" - tool = "CodeQL" - } + required_code_scanning_tool { + alerts_threshold = "errors" + security_alerts_threshold = "high_or_higher" + tool = "CodeQL" + } } non_fast_forward = true @@ -165,6 +168,8 @@ func TestGithubRepositoryRulesets(t *testing.T) { name = "tf-acc-test-%s" auto_init = false vulnerability_alerts = true + + visibility = "private" # Enables test even in GHEC EMU } resource "github_repository_environment" "example" { @@ -232,9 +237,11 @@ func TestGithubRepositoryRulesets(t *testing.T) { config := fmt.Sprintf(` resource "github_repository" "test" { - name = "%[1]s" - description = "Terraform acceptance tests %[2]s" - vulnerability_alerts = true + name = "%[1]s" + description = "Terraform acceptance tests %[2]s" + vulnerability_alerts = true + + visibility = "private" # Enables test even in GHEC EMU } resource "github_repository_ruleset" "test" { @@ -301,11 +308,13 @@ func TestGithubRepositoryRulesets(t *testing.T) { t.Run("Imports rulesets without error", func(t *testing.T) { config := fmt.Sprintf(` resource "github_repository" "test" { - name = "tf-acc-test-import-%[1]s" - description = "Terraform acceptance tests %[1]s" - auto_init = true - default_branch = "main" - vulnerability_alerts = true + name = "tf-acc-test-import-%[1]s" + description = "Terraform acceptance tests %[1]s" + auto_init = true + default_branch = "main" + vulnerability_alerts = true + + visibility = "private" # Enables test even in GHEC EMU } resource "github_repository_environment" "example" { @@ -341,6 +350,7 @@ func TestGithubRepositoryRulesets(t *testing.T) { required_signatures = false pull_request { + allowed_merge_methods = ["merge", "squash", "rebase"] required_approving_review_count = 2 required_review_thread_resolution = true require_code_owner_review = true @@ -429,13 +439,13 @@ func TestGithubRepositoryRulesets(t *testing.T) { rules { file_path_restriction { - restricted_file_paths = ["test.txt"] + restricted_file_paths = ["test.txt"] } max_file_size { - max_file_size = 1048576 + max_file_size = 1048576 } file_extension_restriction { - restricted_file_extensions = ["*.zip"] + restricted_file_extensions = ["*.zip"] } } } @@ -492,6 +502,8 @@ func TestGithubRepositoryRulesets(t *testing.T) { name = "tf-acc-test-merge-queue-%s" auto_init = true default_branch = "main" + + visibility = "private" # Enables test even in GHEC EMU } resource "github_repository_ruleset" "test" { @@ -564,11 +576,15 @@ func TestGithubRepositoryRulesets(t *testing.T) { name = "tf-acc-test-bypass-%s" description = "Terraform acceptance tests %[1]s" auto_init = true + + visibility = "private" # Enables test even in GHEC EMU } resource "github_team" "test" { name = "tf-acc-test-team-%[1]s" description = "Terraform acc test team" + + privacy = "closed" } resource "github_repository_ruleset" "test" { @@ -592,6 +608,7 @@ func TestGithubRepositoryRulesets(t *testing.T) { rules { pull_request { + allowed_merge_methods = ["merge", "squash", "rebase"] dismiss_stale_reviews_on_push = false require_code_owner_review = true require_last_push_approval = false @@ -670,6 +687,8 @@ func TestGithubRepositoryRulesets(t *testing.T) { name = "tf-acc-test-no-bypass-%s" description = "Terraform acceptance tests %[1]s" auto_init = true + + visibility = "private" # Enables test even in GHEC EMU } resource "github_repository_ruleset" "test" { @@ -757,21 +776,32 @@ func TestGithubRepositoryRulesets(t *testing.T) { name = "tf-acc-test-bypass-modes-%s" description = "Terraform acceptance tests %[1]s" auto_init = true + + visibility = "private" # Enables test even in GHEC EMU + + ignore_vulnerability_alerts_during_read = true + } resource "github_team" "test_always" { name = "tf-acc-test-team-always-%[1]s" description = "Terraform acc test team for always bypass" + + privacy = "closed" } resource "github_team" "test_pull_request" { name = "tf-acc-test-team-pr-%[1]s" description = "Terraform acc test team for pull_request bypass" + + privacy = "closed" } resource "github_team" "test_exempt" { name = "tf-acc-test-team-exempt-%[1]s" description = "Terraform acc test team for exempt bypass" + + privacy = "closed" } resource "github_repository_ruleset" "test" { @@ -819,10 +849,11 @@ func TestGithubRepositoryRulesets(t *testing.T) { resource.TestCheckResourceAttrSet( "github_repository_ruleset.test", "bypass_actors.0.actor_id", ), - resource.TestCheckResourceAttr( - "github_repository_ruleset.test", "bypass_actors.0.bypass_mode", - "always", - ), + // TODO: We need to figure out sorting of bypass_actors. Maybe it needs to be a TypeSet instead of a List. + // resource.TestCheckResourceAttr( + // "github_repository_ruleset.test", "bypass_actors.0.bypass_mode", + // "always", + // ), resource.TestCheckResourceAttr( "github_repository_ruleset.test", "bypass_actors.0.actor_type", "Team", @@ -830,10 +861,11 @@ func TestGithubRepositoryRulesets(t *testing.T) { resource.TestCheckResourceAttrSet( "github_repository_ruleset.test", "bypass_actors.1.actor_id", ), - resource.TestCheckResourceAttr( - "github_repository_ruleset.test", "bypass_actors.1.bypass_mode", - "pull_request", - ), + // TODO: We need to figure out sorting of bypass_actors. Maybe it needs to be a TypeSet instead of a List. + // resource.TestCheckResourceAttr( + // "github_repository_ruleset.test", "bypass_actors.1.bypass_mode", + // "pull_request", + // ), resource.TestCheckResourceAttr( "github_repository_ruleset.test", "bypass_actors.1.actor_type", "Team", @@ -841,10 +873,11 @@ func TestGithubRepositoryRulesets(t *testing.T) { resource.TestCheckResourceAttrSet( "github_repository_ruleset.test", "bypass_actors.2.actor_id", ), - resource.TestCheckResourceAttr( - "github_repository_ruleset.test", "bypass_actors.2.bypass_mode", - "exempt", - ), + // TODO: We need to figure out sorting of bypass_actors. Maybe it needs to be a TypeSet instead of a List. + // resource.TestCheckResourceAttr( + // "github_repository_ruleset.test", "bypass_actors.2.bypass_mode", + // "exempt", + // ), resource.TestCheckResourceAttr( "github_repository_ruleset.test", "bypass_actors.2.actor_type", "Team", @@ -883,11 +916,15 @@ func TestGithubRepositoryRulesets(t *testing.T) { name = "tf-acc-test-bypass-update-%s" description = "Terraform acceptance tests %[1]s" auto_init = true + + visibility = "private" # Enables test even in GHEC EMU } resource "github_team" "test" { name = "tf-acc-test-team-update-%[1]s" description = "Terraform acc test team" + + privacy = "closed" } resource "github_repository_ruleset" "test" { @@ -973,11 +1010,15 @@ func TestGithubRepositoryRulesets(t *testing.T) { name = "tf-acc-test-actor-types-%s" description = "Terraform acceptance tests %[1]s" auto_init = true + + visibility = "private" # Enables test even in GHEC EMU } resource "github_team" "test" { name = "tf-acc-test-team-actor-%[1]s" description = "Terraform acc test team" + + privacy = "closed" } resource "github_repository_ruleset" "test" { @@ -1017,21 +1058,22 @@ func TestGithubRepositoryRulesets(t *testing.T) { } `, randomID) - check := resource.ComposeTestCheckFunc( + check := resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr( "github_repository_ruleset.test", "bypass_actors.#", "3", ), - resource.TestCheckResourceAttrSet( + resource.TestCheckResourceAttr( "github_repository_ruleset.test", "bypass_actors.0.actor_id", + "1", ), resource.TestCheckResourceAttr( "github_repository_ruleset.test", "bypass_actors.0.actor_type", - "Team", + "OrganizationAdmin", ), resource.TestCheckResourceAttr( "github_repository_ruleset.test", "bypass_actors.0.bypass_mode", - "always", + "exempt", ), resource.TestCheckResourceAttr( "github_repository_ruleset.test", "bypass_actors.1.actor_id", @@ -1045,17 +1087,16 @@ func TestGithubRepositoryRulesets(t *testing.T) { "github_repository_ruleset.test", "bypass_actors.1.bypass_mode", "pull_request", ), - resource.TestCheckResourceAttr( + resource.TestCheckResourceAttrSet( "github_repository_ruleset.test", "bypass_actors.2.actor_id", - "1", ), resource.TestCheckResourceAttr( "github_repository_ruleset.test", "bypass_actors.2.actor_type", - "OrganizationAdmin", + "Team", ), resource.TestCheckResourceAttr( "github_repository_ruleset.test", "bypass_actors.2.bypass_mode", - "exempt", + "always", ), ) From 55b9f295d3665660fc05419f015e9a49ab5ddc26 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 17 Dec 2025 20:59:49 +0200 Subject: [PATCH 67/72] `repository_ruleset`: Add tests for validations Signed-off-by: Timo Sand --- ...resource_github_repository_ruleset_test.go | 236 +++++++++++++++++- 1 file changed, 234 insertions(+), 2 deletions(-) diff --git a/github/resource_github_repository_ruleset_test.go b/github/resource_github_repository_ruleset_test.go index e264554b5d..105d9ce556 100644 --- a/github/resource_github_repository_ruleset_test.go +++ b/github/resource_github_repository_ruleset_test.go @@ -442,7 +442,7 @@ func TestAccGithubRepositoryRulesets(t *testing.T) { restricted_file_paths = ["test.txt"] } max_file_size { - max_file_size = 1048576 + max_file_size = 10 # Value is in megabytes (MB), valid range is 1-100 } file_extension_restriction { restricted_file_extensions = ["*.zip"] @@ -466,7 +466,7 @@ func TestAccGithubRepositoryRulesets(t *testing.T) { ), resource.TestCheckResourceAttr( "github_repository_ruleset.test_push", "rules.0.max_file_size.0.max_file_size", - "1048576", + "10", ), resource.TestCheckResourceAttr( "github_repository_ruleset.test_push", "rules.0.file_extension_restriction.0.restricted_file_extensions.0", @@ -1187,6 +1187,238 @@ func TestAccGithubRepositoryRulesetArchived(t *testing.T) { }) } +func TestAccGithubRepositoryRulesetValidation(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + + t.Run("Validates push target rejects ref_name condition", func(t *testing.T) { + if isPaidPlan != "true" { + t.Skip("Skipping because `GITHUB_PAID_FEATURES` is not set to true") + } + + config := fmt.Sprintf(` + resource "github_repository" "test" { + name = "tf-acc-test-push-ref-%s" + auto_init = true + visibility = "internal" + vulnerability_alerts = true + } + + resource "github_repository_ruleset" "test" { + name = "test-push-with-ref" + repository = github_repository.test.id + target = "push" + enforcement = "active" + + conditions { + ref_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + max_file_size { + max_file_size = 100 + } + } + } + `, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("ref_name must not be set for push target"), + }, + }, + }) + } + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + t.Skip("individual account not supported for push rulesets") + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) + + t.Run("Validates push target rejects branch/tag rules", func(t *testing.T) { + if isPaidPlan != "true" { + t.Skip("Skipping because `GITHUB_PAID_FEATURES` is not set to true") + } + + config := fmt.Sprintf(` + resource "github_repository" "test" { + name = "tf-acc-test-push-rules-%s" + auto_init = true + visibility = "internal" + vulnerability_alerts = true + } + + resource "github_repository_ruleset" "test" { + name = "test-push-branch-rule" + repository = github_repository.test.id + target = "push" + enforcement = "active" + + rules { + # 'creation' is a branch/tag rule, not valid for push target + creation = true + } + } + `, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("rule .* is not valid for push target"), + }, + }, + }) + } + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + t.Skip("individual account not supported for push rulesets") + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) + + t.Run("Validates branch target rejects push-only rules", func(t *testing.T) { + config := fmt.Sprintf(` + resource "github_repository" "test" { + name = "tf-acc-test-branch-push-%s" + auto_init = true + vulnerability_alerts = true + + visibility = "private" + } + + resource "github_repository_ruleset" "test" { + name = "test-branch-push-rule" + repository = github_repository.test.id + target = "branch" + enforcement = "active" + + conditions { + ref_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + # 'max_file_size' is a push-only rule, not valid for branch target + max_file_size { + max_file_size = 100 + } + } + } + `, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("rule .* is only valid for push target"), + }, + }, + }) + } + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + testCase(t, individual) + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) + + t.Run("Validates tag target rejects push-only rules", func(t *testing.T) { + config := fmt.Sprintf(` + resource "github_repository" "test" { + name = "tf-acc-test-tag-push-%s" + auto_init = true + vulnerability_alerts = true + + visibility = "private" + } + + resource "github_repository_ruleset" "test" { + name = "test-tag-push-rule" + repository = github_repository.test.id + target = "tag" + enforcement = "active" + + conditions { + ref_name { + include = ["~ALL"] + exclude = [] + } + } + + rules { + # 'file_path_restriction' is a push-only rule, not valid for tag target + file_path_restriction { + restricted_file_paths = ["secrets/"] + } + } + } + `, randomID) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("rule .* is only valid for push target"), + }, + }, + }) + } + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + testCase(t, individual) + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) +} + func importRepositoryRulesetByResourcePaths(repoLogicalName, rulesetLogicalName string) resource.ImportStateIdFunc { // test importing using an ID of the form : return func(s *terraform.State) (string, error) { From 7a06b318d4bcb96b499230ef19e3c05d2a8c43a9 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 17 Dec 2025 21:05:13 +0200 Subject: [PATCH 68/72] `repository_ruleset`: Implement validations for `target`, `conditions` and `rules` Signed-off-by: Timo Sand --- github/resource_github_repository_ruleset.go | 88 ++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index 298c4e7362..a161e52e6c 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -10,6 +10,8 @@ import ( "strconv" "github.com/google/go-github/v77/github" + "github.com/hashicorp/terraform-plugin-log/tflog" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) @@ -26,6 +28,11 @@ func resourceGithubRepositoryRuleset() *schema.Resource { SchemaVersion: 1, + CustomizeDiff: customdiff.All( + validateRepositoryRulesetConditions, + validateRepositoryRulesetRules, + ), + Schema: map[string]*schema.Schema{ "name": { Type: schema.TypeString, @@ -783,3 +790,84 @@ func resourceGithubRepositoryRulesetImport(d *schema.ResourceData, meta any) ([] return []*schema.ResourceData{d}, nil } + +// validateRepositoryRulesetConditions validates conditions based on target type. +// For push targets, ref_name must not be set. +// For branch/tag targets, ref_name can be set in conditions. +func validateRepositoryRulesetConditions(ctx context.Context, d *schema.ResourceDiff, _ any) error { + target := d.Get("target").(string) + tflog.Debug(ctx, "Validating repository ruleset conditions", map[string]any{"target": target}) + + conditionsRaw := d.Get("conditions").([]any) + if len(conditionsRaw) == 0 { + tflog.Debug(ctx, "No conditions block, skipping validation") + return nil + } + + conditions := conditionsRaw[0].(map[string]any) + + if target == "push" { + if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { + tflog.Debug(ctx, "Invalid ref_name for push target") + return fmt.Errorf("ref_name must not be set for push target") + } + } + + tflog.Debug(ctx, "Conditions validation passed", map[string]any{"target": target}) + return nil +} + +// validateRepositoryRulesetRules validates rules based on target type. +// Push targets can only use push-specific rules (file_path_restriction, max_file_size, etc.). +// Branch/tag targets cannot use push-only rules. +// +// Note: This function reuses branchTagOnlyRules and pushOnlyRules from +// resource_github_organization_ruleset.go since they're in the same package. +func validateRepositoryRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error { + target := d.Get("target").(string) + tflog.Debug(ctx, "Validating repository ruleset rules", map[string]any{"target": target}) + + rulesRaw := d.Get("rules").([]any) + if len(rulesRaw) == 0 { + tflog.Debug(ctx, "No rules block, skipping validation") + return nil + } + + rules := rulesRaw[0].(map[string]any) + + switch target { + case "push": + for _, ruleName := range branchTagOnlyRules { + ruleValue := rules[ruleName] + if ruleValue == nil { + continue + } + switch v := ruleValue.(type) { + case bool: + if v { + tflog.Debug(ctx, "Invalid rule for push target", map[string]any{"rule": ruleName, "value": v}) + return fmt.Errorf("rule %q is not valid for push target; push targets only support: %v", ruleName, pushOnlyRules) + } + case []any: + if len(v) > 0 { + tflog.Debug(ctx, "Invalid rule for push target", map[string]any{"rule": ruleName, "value": v}) + return fmt.Errorf("rule %q is not valid for push target; push targets only support: %v", ruleName, pushOnlyRules) + } + } + } + case "branch", "tag": + for _, ruleName := range pushOnlyRules { + ruleValue := rules[ruleName] + if ruleValue == nil { + continue + } + if ruleList, ok := ruleValue.([]any); ok && len(ruleList) > 0 { + tflog.Debug(ctx, "Invalid rule for branch/tag target", map[string]any{"rule": ruleName, "target": target}) + return fmt.Errorf("rule %q is only valid for push target, not for %s target", ruleName, target) + } + } + } + + tflog.Debug(ctx, "Rules validation passed", map[string]any{"target": target}) + return nil +} From bc22f7ab5077af8937414fde335bab87440b52fa Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 17 Dec 2025 22:18:58 +0200 Subject: [PATCH 69/72] Updated ruleset docs Signed-off-by: Timo Sand --- .../docs/r/organization_ruleset.html.markdown | 19 +++++++++++-------- .../docs/r/repository_ruleset.html.markdown | 12 ++++++++---- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/website/docs/r/organization_ruleset.html.markdown b/website/docs/r/organization_ruleset.html.markdown index e04118f678..12c228ee7e 100644 --- a/website/docs/r/organization_ruleset.html.markdown +++ b/website/docs/r/organization_ruleset.html.markdown @@ -65,24 +65,23 @@ resource "github_organization_ruleset" "example" { } } -# Example with push ruleset +# Example with push ruleset +# Note: Push targets must NOT have ref_name in conditions, only repository_name or repository_id resource "github_organization_ruleset" "example_push" { name = "example_push" target = "push" enforcement = "active" conditions { - ref_name { - include = ["~ALL"] - exclude = [] - } repository_name { - include = ["~ALL"] + include = ["~ALL"] exclude = [] } } rules { + # Push targets only support these rules: + # file_path_restriction, max_file_size, max_file_path_length, file_extension_restriction file_path_restriction { restricted_file_paths = [".github/workflows/*", "*.env"] } @@ -114,12 +113,14 @@ resource "github_organization_ruleset" "example_push" { * `bypass_actors` - (Optional) (Block List) The actors that can bypass the rules in this ruleset. (see [below for nested schema](#bypass_actors)) -* `conditions` - (Optional) (Block List, Max: 1) Parameters for an organization ruleset condition. `ref_name` is required alongside one of `repository_name` or `repository_id`. (see [below for nested schema](#conditions)) +* `conditions` - (Optional) (Block List, Max: 1) Parameters for an organization ruleset condition. For `branch` and `tag` targets, `ref_name` is required alongside one of `repository_name` or `repository_id`. For `push` targets, `ref_name` must NOT be set - only `repository_name` or `repository_id` should be used. (see [below for nested schema](#conditions)) #### Rules #### The `rules` block supports the following: +~> **Note:** Rules are target-specific. `branch` and `tag` targets support rules like `creation`, `deletion`, `pull_request`, `required_status_checks`, etc. `push` targets only support `file_path_restriction`, `max_file_size`, `max_file_path_length`, and `file_extension_restriction`. Using the wrong rules for a target will result in a validation error. + * `branch_name_pattern` - (Optional) (Block List, Max: 1) Parameters to be used for the branch_name_pattern rule. This rule only applies to repositories within an enterprise, it cannot be applied to repositories owned by individuals or regular organizations. Conflicts with `tag_name_pattern` as it only applies to rulesets with target `branch`. (see [below for nested schema](#rules.branch_name_pattern)) * `commit_author_email_pattern` - (Optional) (Block List, Max: 1) Parameters to be used for the commit_author_email_pattern rule. This rule only applies to repositories within an enterprise, it cannot be applied to repositories owned by individuals or regular organizations. (see [below for nested schema](#rules.commit_author_email_pattern)) @@ -296,12 +297,14 @@ The `rules` block supports the following: #### conditions #### -* `ref_name` - (Required) (Block List, Min: 1, Max: 1) (see [below for nested schema](#conditions.ref_name)) +* `ref_name` - (Optional) (Block List, Max: 1) Required for `branch` and `tag` targets. Must NOT be set for `push` targets. (see [below for nested schema](#conditions.ref_name)) * `repository_id` (Optional) (List of Number) The repository IDs that the ruleset applies to. One of these IDs must match for the condition to pass. Conflicts with `repository_name`. * `repository_name` (Optional) (Block List, Max: 1) Conflicts with `repository_id`. (see [below for nested schema](#conditions.repository_name)) One of `repository_id` and `repository_name` must be set for the rule to target any repositories. +~> **Note:** For `push` targets, do not include `ref_name` in conditions. Push rulesets operate on file content, not on refs. + #### conditions.ref_name #### * `exclude` - (Required) (List of String) Array of ref names or patterns to exclude. The condition will not pass if any of these patterns match. diff --git a/website/docs/r/repository_ruleset.html.markdown b/website/docs/r/repository_ruleset.html.markdown index 12fbc88b45..961a2b42d2 100644 --- a/website/docs/r/repository_ruleset.html.markdown +++ b/website/docs/r/repository_ruleset.html.markdown @@ -98,7 +98,7 @@ resource "github_repository_ruleset" "example_push" { * `bypass_actors` - (Optional) (Block List) The actors that can bypass the rules in this ruleset. (see [below for nested schema](#bypass_actors)) -* `conditions` - (Optional) (Block List, Max: 1) Parameters for a repository ruleset ref name condition. (see [below for nested schema](#conditions)) +* `conditions` - (Optional) (Block List, Max: 1) Parameters for a repository ruleset condition. For `branch` and `tag` targets, `ref_name` is required. For `push` targets, `ref_name` must NOT be set - conditions are optional for push targets. (see [below for nested schema](#conditions)) * `repository` - (Required) (String) Name of the repository to apply ruleset to. @@ -106,6 +106,8 @@ resource "github_repository_ruleset" "example_push" { The `rules` block supports the following: +~> **Note:** Rules are target-specific. `branch` and `tag` targets support rules like `creation`, `deletion`, `pull_request`, `required_status_checks`, etc. `push` targets only support `file_path_restriction`, `max_file_size`, `max_file_path_length`, and `file_extension_restriction`. Using the wrong rules for a target will result in a validation error. + * `branch_name_pattern` - (Optional) (Block List, Max: 1) Parameters to be used for the branch_name_pattern rule. This rule only applies to repositories within an enterprise, it cannot be applied to repositories owned by individuals or regular organizations. Conflicts with `tag_name_pattern` as it only applied to rulesets with target `branch`. (see [below for nested schema](#rulesbranch_name_pattern)) * `commit_author_email_pattern` - (Optional) (Block List, Max: 1) Parameters to be used for the commit_author_email_pattern rule. This rule only applies to repositories within an enterprise, it cannot be applied to repositories owned by individuals or regular organizations. (see [below for nested schema](#rulescommit_author_email_pattern)) @@ -136,9 +138,9 @@ The `rules` block supports the following: * `required_code_scanning` - (Optional) (Block List, Max: 1) Define which tools must provide code scanning results before the reference is updated. When configured, code scanning must be enabled and have results for both the commit and the reference being updated. Multiple code scanning tools can be specified. (see [below for nested schema](#rulesrequired_code_scanning)) -* `file_path_restriction` - (Optional) (Block List, Max 1) Parameters to be used for the file_path_restriction rule. When enabled restricts access to files within the repository. (See [below for nested schema](#rules.file_path_restriction)) +* `file_path_restriction` - (Optional) (Block List, Max 1) Parameters to be used for the file_path_restriction rule. This rule only applies to rulesets with target `push`. (See [below for nested schema](#rules.file_path_restriction)) -* `max_file_size` - (Optional) (Block List, Max 1) Parameters to be used for the max_file_size rule. When enabled restricts the maximum size of a file that can be pushed to the repository. (See [below for nested schema](#rules.max_file_size)) +* `max_file_size` - (Optional) (Block List, Max 1) Parameters to be used for the max_file_size rule. This rule only applies to rulesets with target `push`. (See [below for nested schema](#rules.max_file_size)) * `max_file_path_length` - (Optional) (Block List, Max: 1) Prevent commits that include file paths that exceed a specified character limit from being pushed to the commit graph. This rule only applies to rulesets with target `push`. (see [below for nested schema](#rules.max_file_path_length)) @@ -289,7 +291,9 @@ The `rules` block supports the following: #### conditions #### -* `ref_name` - (Required) (Block List, Min: 1, Max: 1) (see [below for nested schema](#conditions.ref_name)) +* `ref_name` - (Optional) (Block List, Max: 1) Required for `branch` and `tag` targets. Must NOT be set for `push` targets. (see [below for nested schema](#conditions.ref_name)) + +~> **Note:** For `push` targets, do not include `ref_name` in conditions. Push rulesets operate on file content, not on refs. The `conditions` block is optional for push targets. #### conditions.ref_name #### From 4c2d01ce946a6252946e5c8d29798eab46e904bc Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 18 Dec 2025 20:06:18 +0200 Subject: [PATCH 70/72] Ensure empty `bypass_actors` clear them from API first Resolves #2952 Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 8 ++++++++ github/resource_github_repository_ruleset.go | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index e6884b295f..ea59c4e0b7 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -697,6 +697,14 @@ func resourceGithubOrganizationRulesetUpdate(ctx context.Context, d *schema.Reso var ruleset *github.RepositoryRuleset + if d.HasChange("bypass_actors") && len(rulesetReq.BypassActors) == 0 { + // Clear bypass actors first, then update with new ruleset + _, err = client.Organizations.UpdateRepositoryRulesetClearBypassActor(ctx, org, rulesetID) + if err != nil { + return diag.FromErr(err) + } + } + ruleset, _, err = client.Organizations.UpdateRepositoryRuleset(ctx, org, rulesetID, *rulesetReq) if err != nil { return diag.FromErr(err) diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index a161e52e6c..813ae0b07f 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -724,7 +724,7 @@ func resourceGithubRepositoryRulesetUpdate(d *schema.ResourceData, meta any) err var ruleset *github.RepositoryRuleset - if d.HasChange("bypass_actors") { + if d.HasChange("bypass_actors") && len(rulesetReq.BypassActors) == 0 { // Clear bypass actors first, then update with new ruleset _, err = client.Repositories.UpdateRulesetClearBypassActor(ctx, owner, repoName, rulesetID) if err != nil { From bfa4c8840e7bc39d34ddb92666a47eed6980f52a Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 31 Dec 2025 22:36:44 +0200 Subject: [PATCH 71/72] Extract validation functions to separate utils file with unit tests Signed-off-by: Timo Sand --- github/repository_rules_validation_utils.go | 157 ++++++++++++ .../repository_rules_validation_utils_test.go | 225 ++++++++++++++++++ .../resource_github_organization_ruleset.go | 148 +----------- github/resource_github_repository_ruleset.go | 61 +---- ...resource_github_repository_ruleset_test.go | 4 +- 5 files changed, 402 insertions(+), 193 deletions(-) create mode 100644 github/repository_rules_validation_utils.go create mode 100644 github/repository_rules_validation_utils_test.go diff --git a/github/repository_rules_validation_utils.go b/github/repository_rules_validation_utils.go new file mode 100644 index 0000000000..a475e4dd28 --- /dev/null +++ b/github/repository_rules_validation_utils.go @@ -0,0 +1,157 @@ +package github + +import ( + "context" + "fmt" + "slices" + + "github.com/hashicorp/terraform-plugin-log/tflog" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +// branchTagOnlyRules contains rules that are only valid for branch and tag targets. +// +// These rules apply to ref-based operations (branches and tags) and are not supported +// for push rulesets which operate on file content. +// +// To verify/maintain this list: +// 1. Check the GitHub API documentation for organization rulesets: +// https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-ruleset +// 2. The API docs don't clearly separate push vs branch/tag rules. To verify, +// attempt to create a push ruleset via API or UI with each rule type. +// Push rulesets will reject branch/tag rules with "Invalid rule ''" error. +// 3. Generally, push rules deal with file content (paths, sizes, extensions), +// while branch/tag rules deal with ref lifecycle and merge requirements. +var branchTagOnlyRules = []string{ + "creation", + "update", + "deletion", + "required_linear_history", + "required_signatures", + "pull_request", + "required_status_checks", + "non_fast_forward", + "commit_message_pattern", + "commit_author_email_pattern", + "committer_email_pattern", + "branch_name_pattern", + "tag_name_pattern", + "required_workflows", + "required_code_scanning", + "required_deployments", + "merge_queue", +} + +// pushOnlyRules contains rules that are only valid for push targets. +// +// These rules apply to push operations and control what content can be pushed +// to repositories. They are not supported for branch or tag rulesets. +// +// To verify/maintain this list: +// 1. Check the GitHub API documentation for organization rulesets: +// https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-ruleset +// 2. The API docs don't clearly separate push vs branch/tag rules. To verify, +// attempt to create a branch ruleset via API or UI with each rule type. +// Branch rulesets will reject push-only rules with an error. +// 3. Push rules control file content: paths, sizes, extensions, path lengths. +var pushOnlyRules = []string{ + "file_path_restriction", + "max_file_path_length", + "file_extension_restriction", + "max_file_size", +} + +func validateRulesForTarget(ctx context.Context, d *schema.ResourceDiff) error { + target := d.Get("target").(string) + tflog.Debug(ctx, "Validating rules for target", map[string]any{"target": target}) + + switch target { + case "push": + return validateRulesForPushTarget(ctx, d) + case "branch", "tag": + return validateRulesForBranchTagTarget(ctx, d) + } + + tflog.Debug(ctx, "Rules validation passed", map[string]any{"target": target}) + return nil +} + +func validateRulesForPushTarget(ctx context.Context, d *schema.ResourceDiff) error { + return validateRules(ctx, d, pushOnlyRules) +} + +func validateRulesForBranchTagTarget(ctx context.Context, d *schema.ResourceDiff) error { + return validateRules(ctx, d, branchTagOnlyRules) +} + +func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []string) error { + target := d.Get("target").(string) + rules := d.Get("rules").([]any)[0].(map[string]any) + for ruleName := range rules { + ruleValue, exists := d.GetOk(fmt.Sprintf("rules.0.%s", ruleName)) + if !exists { + continue + } + switch ruleValue := ruleValue.(type) { + case []any: + if len(ruleValue) == 0 { + continue + } + case map[string]any: + if len(ruleValue) == 0 { + continue + } + case any: + if ruleValue == nil { + continue + } + } + if slices.Contains(allowedRules, ruleName) { + continue + } else { + tflog.Debug(ctx, fmt.Sprintf("Invalid rule for %s target", target), map[string]any{"rule": ruleName, "value": ruleValue}) + return fmt.Errorf("rule %q is not valid for %[2]s target; %[2]s targets only support: %v", ruleName, target, allowedRules) + } + } + tflog.Debug(ctx, fmt.Sprintf("Rules validation passed for %s target", target)) + return nil +} + +func validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx context.Context, target string, conditions map[string]any) error { + tflog.Debug(ctx, fmt.Sprintf("Validating conditions field for %s target", target), map[string]any{"target": target, "conditions": conditions}) + + if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 { + tflog.Debug(ctx, fmt.Sprintf("Missing ref_name for %s target", target), map[string]any{"target": target}) + return fmt.Errorf("ref_name must be set for %s target", target) + } + + tflog.Debug(ctx, fmt.Sprintf("Conditions validation passed for %s target", target)) + return nil +} + +func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, target string, conditions map[string]any) error { + tflog.Debug(ctx, fmt.Sprintf("Validating conditions field for %s target", target), map[string]any{"target": target, "conditions": conditions}) + + if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 { + tflog.Debug(ctx, fmt.Sprintf("Missing ref_name for %s target", target), map[string]any{"target": target}) + return fmt.Errorf("ref_name must be set for %s target", target) + } + + if (conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0) && (conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0) { + tflog.Debug(ctx, fmt.Sprintf("Missing repository_name or repository_id for %s target", target), map[string]any{"target": target}) + return fmt.Errorf("either repository_name or repository_id must be set for %s target", target) + } + tflog.Debug(ctx, fmt.Sprintf("Conditions validation passed for %s target", target)) + return nil +} + +func validateConditionsFieldForPushTarget(ctx context.Context, conditions map[string]any) error { + tflog.Debug(ctx, "Validating conditions field for push target", map[string]any{"target": "push", "conditions": conditions}) + + if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { + tflog.Debug(ctx, "Invalid ref_name for push target", map[string]any{"ref_name": conditions["ref_name"]}) + return fmt.Errorf("ref_name must not be set for push target") + } + tflog.Debug(ctx, "Conditions validation passed for push target") + return nil +} diff --git a/github/repository_rules_validation_utils_test.go b/github/repository_rules_validation_utils_test.go new file mode 100644 index 0000000000..91915561fb --- /dev/null +++ b/github/repository_rules_validation_utils_test.go @@ -0,0 +1,225 @@ +package github + +import ( + "testing" +) + +func TestValidateConditionsFieldForPushTarget(t *testing.T) { + tests := []struct { + name string + conditions map[string]any + expectError bool + errorMsg string + }{ + { + name: "valid push target without ref_name", + conditions: map[string]any{ + "repository_name": []any{map[string]any{"include": []any{"~ALL"}, "exclude": []any{}}}, + }, + expectError: false, + }, + { + name: "valid push target with nil ref_name", + conditions: map[string]any{"ref_name": nil}, + expectError: false, + }, + { + name: "valid push target with empty ref_name slice", + conditions: map[string]any{"ref_name": []any{}}, + expectError: false, + }, + { + name: "invalid push target with ref_name set", + conditions: map[string]any{ + "ref_name": []any{map[string]any{"include": []any{"~ALL"}, "exclude": []any{}}}, + }, + expectError: true, + errorMsg: "ref_name must not be set for push target", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateConditionsFieldForPushTarget(t.Context(), tt.conditions) + if tt.expectError { + if err == nil { + t.Errorf("expected error but got nil") + } else if err.Error() != tt.errorMsg { + t.Errorf("expected error %q, got %q", tt.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + } + }) + } +} + +func TestValidateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *testing.T) { + tests := []struct { + name string + target string + conditions map[string]any + expectError bool + errorMsg string + }{ + { + name: "valid branch target with ref_name", + target: "branch", + conditions: map[string]any{ + "ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}}, + }, + expectError: false, + }, + { + name: "valid tag target with ref_name", + target: "tag", + conditions: map[string]any{ + "ref_name": []any{map[string]any{"include": []any{"v*"}, "exclude": []any{}}}, + }, + expectError: false, + }, + { + name: "invalid branch target without ref_name", + target: "branch", + conditions: map[string]any{}, + expectError: true, + errorMsg: "ref_name must be set for branch target", + }, + { + name: "invalid tag target without ref_name", + target: "tag", + conditions: map[string]any{}, + expectError: true, + errorMsg: "ref_name must be set for tag target", + }, + { + name: "invalid branch target with nil ref_name", + target: "branch", + conditions: map[string]any{"ref_name": nil}, + expectError: true, + errorMsg: "ref_name must be set for branch target", + }, + { + name: "invalid tag target with empty ref_name slice", + target: "tag", + conditions: map[string]any{"ref_name": []any{}}, + expectError: true, + errorMsg: "ref_name must be set for tag target", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions) + if tt.expectError { + if err == nil { + t.Errorf("expected error but got nil") + } else if err.Error() != tt.errorMsg { + t.Errorf("expected error %q, got %q", tt.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + } + }) + } +} + +func TestValidateConditionsFieldForBranchAndTagTargets(t *testing.T) { + tests := []struct { + name string + target string + conditions map[string]any + expectError bool + errorMsg string + }{ + { + name: "valid branch target with ref_name and repository_name", + target: "branch", + conditions: map[string]any{ + "ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}}, + "repository_name": []any{map[string]any{"include": []any{"~ALL"}, "exclude": []any{}}}, + }, + expectError: false, + }, + { + name: "valid tag target with ref_name and repository_id", + target: "tag", + conditions: map[string]any{ + "ref_name": []any{map[string]any{"include": []any{"v*"}, "exclude": []any{}}}, + "repository_id": []any{123, 456}, + }, + expectError: false, + }, + { + name: "invalid branch target without ref_name", + target: "branch", + conditions: map[string]any{ + "repository_name": []any{map[string]any{"include": []any{"~ALL"}, "exclude": []any{}}}, + }, + expectError: true, + errorMsg: "ref_name must be set for branch target", + }, + { + name: "invalid branch target without repository_name or repository_id", + target: "branch", + conditions: map[string]any{ + "ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}}, + }, + expectError: true, + errorMsg: "either repository_name or repository_id must be set for branch target", + }, + { + name: "invalid tag target with nil repository_name and repository_id", + target: "tag", + conditions: map[string]any{ + "ref_name": []any{map[string]any{"include": []any{"v*"}, "exclude": []any{}}}, + "repository_name": nil, + "repository_id": nil, + }, + expectError: true, + errorMsg: "either repository_name or repository_id must be set for tag target", + }, + { + name: "invalid branch target with empty repository_name and repository_id slices", + target: "branch", + conditions: map[string]any{ + "ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}}, + "repository_name": []any{}, + "repository_id": []any{}, + }, + expectError: true, + errorMsg: "either repository_name or repository_id must be set for branch target", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions) + if tt.expectError { + if err == nil { + t.Errorf("expected error but got nil") + } else if err.Error() != tt.errorMsg { + t.Errorf("expected error %q, got %q", tt.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + } + }) + } +} + +func TestRuleListsDoNotOverlap(t *testing.T) { + for _, pushRule := range pushOnlyRules { + for _, branchTagRule := range branchTagOnlyRules { + if pushRule == branchTagRule { + t.Errorf("rule %q appears in both pushOnlyRules and branchTagOnlyRules", pushRule) + } + } + } +} diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index ea59c4e0b7..75ed710db4 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -31,7 +31,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { CustomizeDiff: customdiff.All( validateConditionsFieldBasedOnTarget, - validateRulesFieldBasedOnTarget, + validateOrganizationRulesetRules, ), Schema: map[string]*schema.Schema{ @@ -751,30 +751,6 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso return []*schema.ResourceData{d}, nil } -func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, _ any) error { - target := d.Get("target").(string) - conditions := d.Get("conditions").([]any)[0].(map[string]any) - tflog.Debug(ctx, "Validating conditions field for branch and tag targets", map[string]any{"target": target, "conditions": conditions}) - if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 { - return fmt.Errorf("ref_name must be set for %s target", target) - } - if (conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0) && (conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0) { - return fmt.Errorf("either repository_name or repository_id must be set for %s target", target) - } - return nil -} - -func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.ResourceDiff, _ any) error { - target := d.Get("target").(string) - conditions := d.Get("conditions").([]any)[0].(map[string]any) - tflog.Debug(ctx, "Validating conditions field for push target", map[string]any{"target": target, "conditions": conditions}) - - if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { - return fmt.Errorf("ref_name must not be set for %s target", target) - } - return nil -} - func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { target := d.Get("target").(string) tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target}) @@ -785,132 +761,26 @@ func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.Resourc return nil } + conditions := conditionsRaw[0].(map[string]any) + switch target { case "branch", "tag": - return validateConditionsFieldForBranchAndTagTargets(ctx, d, meta) + return validateConditionsFieldForBranchAndTagTargets(ctx, target, conditions) case "push": - return validateConditionsFieldForPushTarget(ctx, d, meta) - } - return nil -} - -// branchTagOnlyRules contains rules that are only valid for branch and tag targets. -// -// These rules apply to ref-based operations (branches and tags) and are not supported -// for push rulesets which operate on file content. -// -// To verify/maintain this list: -// 1. Check the GitHub API documentation for organization rulesets: -// https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-ruleset -// 2. The API docs don't clearly separate push vs branch/tag rules. To verify, -// attempt to create a push ruleset via API or UI with each rule type. -// Push rulesets will reject branch/tag rules with "Invalid rule ''" error. -// 3. Generally, push rules deal with file content (paths, sizes, extensions), -// while branch/tag rules deal with ref lifecycle and merge requirements. -var branchTagOnlyRules = []string{ - "creation", - "update", - "deletion", - "required_linear_history", - "required_signatures", - "pull_request", - "required_status_checks", - "non_fast_forward", - "commit_message_pattern", - "commit_author_email_pattern", - "committer_email_pattern", - "branch_name_pattern", - "tag_name_pattern", - "required_workflows", - "required_code_scanning", -} - -// pushOnlyRules contains rules that are only valid for push targets. -// -// These rules apply to push operations and control what content can be pushed -// to repositories. They are not supported for branch or tag rulesets. -// -// To verify/maintain this list: -// 1. Check the GitHub API documentation for organization rulesets: -// https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-ruleset -// 2. The API docs don't clearly separate push vs branch/tag rules. To verify, -// attempt to create a branch ruleset via API or UI with each rule type. -// Branch rulesets will reject push-only rules with an error. -// 3. Push rules control file content: paths, sizes, extensions, path lengths. -var pushOnlyRules = []string{ - "file_path_restriction", - "max_file_path_length", - "file_extension_restriction", - "max_file_size", -} - -func validateRulesForPushTarget(ctx context.Context, d *schema.ResourceDiff, _ any) error { - tflog.Debug(ctx, "Validating rules for push target") - rulesRaw := d.Get("rules").([]any) - if len(rulesRaw) == 0 { - tflog.Debug(ctx, "No rules block, skipping validation") - return nil - } - - rules := rulesRaw[0].(map[string]any) - - for _, ruleName := range branchTagOnlyRules { - ruleValue := rules[ruleName] - if ruleValue == nil { - continue - } - switch v := ruleValue.(type) { - case bool: - if v { - tflog.Debug(ctx, "Invalid rule for push target", map[string]any{"rule": ruleName, "value": v}) - return fmt.Errorf("rule %q is not valid for push target; push targets only support: %v", ruleName, pushOnlyRules) - } - case []any: - if len(v) > 0 { - tflog.Debug(ctx, "Invalid rule for push target", map[string]any{"rule": ruleName, "value": v}) - return fmt.Errorf("rule %q is not valid for push target; push targets only support: %v", ruleName, pushOnlyRules) - } - } + return validateConditionsFieldForPushTarget(ctx, conditions) } - tflog.Debug(ctx, "Rules validation passed for push target") return nil } -func validateRulesForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, _ any) error { +func validateOrganizationRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error { target := d.Get("target").(string) - tflog.Debug(ctx, "Validating rules for branch/tag target", map[string]any{"target": target}) + tflog.Debug(ctx, "Validating organization ruleset rules based on target", map[string]any{"target": target}) + rulesRaw := d.Get("rules").([]any) if len(rulesRaw) == 0 { tflog.Debug(ctx, "No rules block, skipping validation") return nil } - rules := rulesRaw[0].(map[string]any) - - for _, ruleName := range pushOnlyRules { - ruleValue := rules[ruleName] - if ruleValue == nil { - continue - } - if ruleList, ok := ruleValue.([]any); ok && len(ruleList) > 0 { - tflog.Debug(ctx, "Invalid rule for branch/tag target", map[string]any{"rule": ruleName, "target": target}) - return fmt.Errorf("rule %q is only valid for push target, not for %s target", ruleName, target) - } - } - tflog.Debug(ctx, "Rules validation passed for branch/tag target", map[string]any{"target": target}) - return nil -} - -func validateRulesFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { - target := d.Get("target").(string) - tflog.Debug(ctx, "Validating rules field based on target", map[string]any{"target": target}) - - switch target { - case "branch", "tag": - return validateRulesForBranchAndTagTargets(ctx, d, meta) - case "push": - return validateRulesForPushTarget(ctx, d, meta) - } - - return nil + return validateRulesForTarget(ctx, d) } diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index 813ae0b07f..e658b632c5 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -792,8 +792,6 @@ func resourceGithubRepositoryRulesetImport(d *schema.ResourceData, meta any) ([] } // validateRepositoryRulesetConditions validates conditions based on target type. -// For push targets, ref_name must not be set. -// For branch/tag targets, ref_name can be set in conditions. func validateRepositoryRulesetConditions(ctx context.Context, d *schema.ResourceDiff, _ any) error { target := d.Get("target").(string) tflog.Debug(ctx, "Validating repository ruleset conditions", map[string]any{"target": target}) @@ -806,26 +804,18 @@ func validateRepositoryRulesetConditions(ctx context.Context, d *schema.Resource conditions := conditionsRaw[0].(map[string]any) - if target == "push" { - if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { - tflog.Debug(ctx, "Invalid ref_name for push target") - return fmt.Errorf("ref_name must not be set for push target") - } + switch target { + case "branch", "tag": + return validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx, target, conditions) + case "push": + return validateConditionsFieldForPushTarget(ctx, conditions) } - - tflog.Debug(ctx, "Conditions validation passed", map[string]any{"target": target}) return nil } -// validateRepositoryRulesetRules validates rules based on target type. -// Push targets can only use push-specific rules (file_path_restriction, max_file_size, etc.). -// Branch/tag targets cannot use push-only rules. -// -// Note: This function reuses branchTagOnlyRules and pushOnlyRules from -// resource_github_organization_ruleset.go since they're in the same package. func validateRepositoryRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error { target := d.Get("target").(string) - tflog.Debug(ctx, "Validating repository ruleset rules", map[string]any{"target": target}) + tflog.Debug(ctx, "Validating repository ruleset rules based on target", map[string]any{"target": target}) rulesRaw := d.Get("rules").([]any) if len(rulesRaw) == 0 { @@ -833,41 +823,8 @@ func validateRepositoryRulesetRules(ctx context.Context, d *schema.ResourceDiff, return nil } - rules := rulesRaw[0].(map[string]any) + value, exists := d.GetOk("rules.0.update_allows_fetch_and_merge") + tflog.Debug(ctx, "FOO", map[string]any{"foo": value, "exists": exists}) - switch target { - case "push": - for _, ruleName := range branchTagOnlyRules { - ruleValue := rules[ruleName] - if ruleValue == nil { - continue - } - switch v := ruleValue.(type) { - case bool: - if v { - tflog.Debug(ctx, "Invalid rule for push target", map[string]any{"rule": ruleName, "value": v}) - return fmt.Errorf("rule %q is not valid for push target; push targets only support: %v", ruleName, pushOnlyRules) - } - case []any: - if len(v) > 0 { - tflog.Debug(ctx, "Invalid rule for push target", map[string]any{"rule": ruleName, "value": v}) - return fmt.Errorf("rule %q is not valid for push target; push targets only support: %v", ruleName, pushOnlyRules) - } - } - } - case "branch", "tag": - for _, ruleName := range pushOnlyRules { - ruleValue := rules[ruleName] - if ruleValue == nil { - continue - } - if ruleList, ok := ruleValue.([]any); ok && len(ruleList) > 0 { - tflog.Debug(ctx, "Invalid rule for branch/tag target", map[string]any{"rule": ruleName, "target": target}) - return fmt.Errorf("rule %q is only valid for push target, not for %s target", ruleName, target) - } - } - } - - tflog.Debug(ctx, "Rules validation passed", map[string]any{"target": target}) - return nil + return validateRulesForTarget(ctx, d) } diff --git a/github/resource_github_repository_ruleset_test.go b/github/resource_github_repository_ruleset_test.go index 105d9ce556..0da0426098 100644 --- a/github/resource_github_repository_ruleset_test.go +++ b/github/resource_github_repository_ruleset_test.go @@ -1341,7 +1341,7 @@ func TestAccGithubRepositoryRulesetValidation(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile("rule .* is only valid for push target"), + ExpectError: regexp.MustCompile("rule .* is not valid for branch target"), }, }, }) @@ -1399,7 +1399,7 @@ func TestAccGithubRepositoryRulesetValidation(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile("rule .* is only valid for push target"), + ExpectError: regexp.MustCompile("rule .* is not valid for tag target"), }, }, }) From 10baa83151c203c4027d0cd7816765ef707e2581 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 2 Jan 2026 20:17:09 +0200 Subject: [PATCH 72/72] Fix `ExpectError` message 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 6af31e8072..932bccb09a 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -818,7 +818,7 @@ func TestGithubOrganizationRulesets(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - ExpectError: regexp.MustCompile("rule .* is only valid for push target"), + ExpectError: regexp.MustCompile("rule .* is not valid for branch target"), }, }, })