From a3526c34ccb2a4a01571d47d90bc9cf8652309ac Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 27 Nov 2025 22:11:15 +0200 Subject: [PATCH 01/56] 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 83c7d0ce9f..5b40a33d6f 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 b5e87b9835ad1f0fa77e32c5e325808014324845 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 27 Nov 2025 22:11:35 +0200 Subject: [PATCH 02/56] Add `CustomizeDiff` logic to validate on `plan` Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 5b40a33d6f..638d1a7a34 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, @@ -855,3 +857,46 @@ 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) + + switch target { + case "branch", "tag": + return validateConditionsFieldForBranchAndTagTargets(d, meta) + case "push": + return validateConditionsFieldForPushTarget(d, meta) + case "repository": + return validateConditionsFieldForRepositoryTarget(d, meta) + } + return nil +} From ebae94b1fb4456bcc8b821b0283a8948ee4dca9e Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 29 Nov 2025 19:44:16 +0200 Subject: [PATCH 03/56] Add first validation test Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 68 +++++++++---------- ...source_github_organization_ruleset_test.go | 35 ++++++++++ 2 files changed, 68 insertions(+), 35 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 638d1a7a34..fc91919d8e 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": { @@ -858,45 +858,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 +func validateConditionsFieldForBranchAndTagTargets(d *schema.ResourceDiff, _ any) 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 validateConditionsFieldForPushTarget(d *schema.ResourceDiff, _ any) 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 validateConditionsFieldForRepositoryTarget(d *schema.ResourceDiff, _ any) 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 any) error { + target := d.Get("target").(string) -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 + switch target { + case "branch", "tag": + return validateConditionsFieldForBranchAndTagTargets(d, meta) + case "push": + return validateConditionsFieldForPushTarget(d, meta) + case "repository": + return validateConditionsFieldForRepositoryTarget(d, meta) + } + return nil } diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index fea3624c6c..f7fe6b6948 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" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" @@ -499,6 +500,40 @@ resource "github_organization_ruleset" "test" { }, }) }) + + t.Run("validates_branch_target_requires_ref_name", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + 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) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + 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"), + }, + }, + }) + }) } func TestOrganizationPushRulesetSupport(t *testing.T) { From 92a6ea121bd28263d7a7b3e00154b848865e6262 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 30 Nov 2025 08:22:36 +0200 Subject: [PATCH 04/56] Add validation error when `conditions` is missing Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 6 ++++ ...source_github_organization_ruleset_test.go | 29 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index fc91919d8e..315dcae56d 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -887,6 +887,12 @@ func validateConditionsFieldForRepositoryTarget(d *schema.ResourceDiff, _ any) e func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) 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 f7fe6b6948..8bce990fdd 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -501,6 +501,35 @@ resource "github_organization_ruleset" "test" { }) }) + t.Run("panics when conditions block is missing for branch target", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + config := fmt.Sprintf(` + resource "github_organization_ruleset" "test" { + name = "test-no-conditions-%s" + target = "branch" + enforcement = "active" + + rules { + creation = true + } + } + `, randomID) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + 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("validates_branch_target_requires_ref_name", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) config := fmt.Sprintf(` From 0fc34a1e7af1559a264a342ee8099cf9ec245541 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 30 Nov 2025 09:23:50 +0200 Subject: [PATCH 05/56] 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 8bce990fdd..e5b58eb439 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -563,6 +563,157 @@ resource "github_organization_ruleset" "test" { }, }) }) + + 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 088cb0824a98503674fe690b2e99645d74115089 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 2 Dec 2025 21:55:23 +0200 Subject: [PATCH 06/56] 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 | 110 +++++------------- 1 file changed, 30 insertions(+), 80 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index e5b58eb439..4f775cace5 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -564,7 +564,8 @@ resource "github_organization_ruleset" "test" { }) }) - t.Run("Validates tag target requires conditions", func(t *testing.T) { + t.Run("validates_tag_target_requires_conditions", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { name = "test-tag-no-conditions-%s" @@ -577,37 +578,20 @@ resource "github_organization_ruleset" "test" { } `, 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`), - }, + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + 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) { + t.Run("validates_push_target_rejects_ref_name", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { name = "test-push-with-ref-%s" @@ -631,37 +615,21 @@ resource "github_organization_ruleset" "test" { } `, 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`), - }, + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + 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) { + t.Run("validates_repository_target_rejects_ref_name", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { name = "test-repo-with-ref-%s" @@ -685,33 +653,15 @@ resource "github_organization_ruleset" "test" { } `, 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`), - }, + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + 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) + }, }) }) } From e618a988696894974be6ffca6bc7a04abbcb7e27 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 2 Dec 2025 21:57:29 +0200 Subject: [PATCH 07/56] Switch `ref_name` to `Optional` as `push` doesn't need a `ref_name` Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 2 +- ...source_github_organization_ruleset_test.go | 23 +++++++++---------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 315dcae56d..63b697de16 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 4f775cace5..1575fc6c8d 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -516,8 +516,8 @@ resource "github_organization_ruleset" "test" { `, randomID) resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessHasPaidOrgs(t) }, - Providers: testAccProviders, + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, @@ -552,13 +552,12 @@ resource "github_organization_ruleset" "test" { `, randomID) resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessHasPaidOrgs(t) }, - Providers: testAccProviders, + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, 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"), }, }, }) @@ -579,8 +578,8 @@ resource "github_organization_ruleset" "test" { `, randomID) resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessHasPaidOrgs(t) }, - Providers: testAccProviders, + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, @@ -616,8 +615,8 @@ resource "github_organization_ruleset" "test" { `, randomID) resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessHasPaidOrgs(t) }, - Providers: testAccProviders, + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, @@ -654,8 +653,8 @@ resource "github_organization_ruleset" "test" { `, randomID) resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessHasPaidOrgs(t) }, - Providers: testAccProviders, + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, Steps: []resource.TestStep{ { Config: config, From 80c6a3993d1f0f489083bf451fe91998b6ffcde7 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 12:34:24 +0200 Subject: [PATCH 08/56] 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 63b697de16..61908a9679 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 { @@ -858,25 +859,26 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso return []*schema.ResourceData{d}, nil } -func validateConditionsFieldForBranchAndTagTargets(d *schema.ResourceDiff, _ any) error { +func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, _ any) 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]any{"conditions": conditions}) } return nil } -func validateConditionsFieldForPushTarget(d *schema.ResourceDiff, _ any) error { +func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.ResourceDiff, _ any) 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]any{"conditions": conditions}) + return fmt.Errorf("ref_name must not be set for push target") } return nil } -func validateConditionsFieldForRepositoryTarget(d *schema.ResourceDiff, _ any) error { +func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.ResourceDiff, _ any) 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]any{"conditions": conditions}) + return fmt.Errorf("ref_name must not be set for repository target") } if conditions["repository_name"] == nil && conditions["repository_id"] == nil { @@ -887,6 +889,7 @@ func validateConditionsFieldForRepositoryTarget(d *schema.ResourceDiff, _ any) e 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}) conditionsRaw := d.Get("conditions").([]any) // Handle empty conditions - branch and tag targets require conditions with ref_name @@ -896,11 +899,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 fe67604b552560aedb7dc8e46d3b4533570e2bf5 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 12:35:10 +0200 Subject: [PATCH 09/56] 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 | 10 ++++++++-- github/resource_github_organization_ruleset_test.go | 4 +--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 61908a9679..a33f5abb08 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -12,7 +12,6 @@ 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 { @@ -862,6 +861,11 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, _ any) error { conditions := d.Get("conditions").([]any)[0].(map[string]any) tflog.Debug(ctx, "Validating conditions field for branch and tag targets", map[string]any{"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 } @@ -870,6 +874,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]any{"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 @@ -879,9 +884,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]any{"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 diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 1575fc6c8d..6e0b51eff4 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -527,7 +527,6 @@ resource "github_organization_ruleset" "test" { }, }, }) - }) t.Run("validates_branch_target_requires_ref_name", func(t *testing.T) { @@ -557,7 +556,7 @@ resource "github_organization_ruleset" "test" { 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"), }, }, }) @@ -624,7 +623,6 @@ resource "github_organization_ruleset" "test" { }, }, }) - }) t.Run("validates_repository_target_rejects_ref_name", func(t *testing.T) { From f85842b13f3115c59ba6eb3f143b3495722d2a7c Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 12:49:48 +0200 Subject: [PATCH 10/56] Remove unnecessary panic test Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 6 ++-- ...source_github_organization_ruleset_test.go | 28 ------------------- 2 files changed, 3 insertions(+), 31 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index a33f5abb08..11a31a2dd5 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -898,9 +898,9 @@ 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) - // 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 6e0b51eff4..4ad30835ae 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -501,34 +501,6 @@ resource "github_organization_ruleset" "test" { }) }) - t.Run("panics when conditions block is missing for branch target", func(t *testing.T) { - randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) - config := fmt.Sprintf(` - resource "github_organization_ruleset" "test" { - name = "test-no-conditions-%s" - target = "branch" - enforcement = "active" - - rules { - creation = true - } - } - `, randomID) - - resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessHasPaidOrgs(t) }, - ProviderFactories: providerFactories, - 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("validates_branch_target_requires_ref_name", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) config := fmt.Sprintf(` From cdad2618f63c4c46310981c6ec8fc20322cb7f5c Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 13:00:34 +0200 Subject: [PATCH 11/56] Improve validation output messages Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 23 +++++++++++-------- ...source_github_organization_ruleset_test.go | 19 ++++++++++----- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 11a31a2dd5..91d2ac2f7d 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -859,36 +859,39 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso } 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{"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 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, _ 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{"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 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, _ 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{"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 { - 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 } @@ -898,8 +901,8 @@ 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 { - tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]interface{}{"target": target}) + if len(conditionsRaw) == 0 { + tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]any{"target": target}) return nil } diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 4ad30835ae..63deabe4fa 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -501,7 +501,7 @@ resource "github_organization_ruleset" "test" { }) }) - 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) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { @@ -528,13 +528,13 @@ resource "github_organization_ruleset" "test" { 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"), }, }, }) }) - t.Run("validates_tag_target_requires_conditions", func(t *testing.T) { + t.Run("validates_tag_target_requires_ref_name_condition", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) config := fmt.Sprintf(` resource "github_organization_ruleset" "test" { @@ -542,6 +542,13 @@ resource "github_organization_ruleset" "test" { target = "tag" enforcement = "active" + conditions { + repository_name { + include = ["~ALL"] + exclude = [] + } + } + rules { creation = true } @@ -554,7 +561,7 @@ resource "github_organization_ruleset" "test" { 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"), }, }, }) @@ -591,7 +598,7 @@ resource "github_organization_ruleset" "test" { 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"), }, }, }) @@ -628,7 +635,7 @@ resource "github_organization_ruleset" "test" { 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 a2df271677d625efa5946e39d389c174d9f6064e Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 3 Dec 2025 17:12:51 +0200 Subject: [PATCH 12/56] Fix condition to require only one of `repository_name` or `repository_id` Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 2 +- github/util_rules.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 91d2ac2f7d..e9bb076e65 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -865,7 +865,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/util_rules.go b/github/util_rules.go index bc8ac1cdd8..1c6c661656 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -573,7 +573,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 d25124996392a46010dd6332f007be653f95aec4 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 01:21:25 +0200 Subject: [PATCH 13/56] 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 e9bb076e65..3204ca50d1 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "regexp" "strconv" "github.com/google/go-github/v82/github" @@ -490,9 +491,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 f14992519cb363780f0d189f74c555d7914289a4 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 02:14:07 +0200 Subject: [PATCH 14/56] 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 63deabe4fa..f03d9a4a74 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -569,8 +569,9 @@ resource "github_organization_ruleset" "test" { t.Run("validates_push_target_rejects_ref_name", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + 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" @@ -590,7 +591,7 @@ resource "github_organization_ruleset" "test" { creation = true } } - `, randomID) + `, resourceName, randomID) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasPaidOrgs(t) }, @@ -606,8 +607,9 @@ resource "github_organization_ruleset" "test" { t.Run("validates_repository_target_rejects_ref_name", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + 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" @@ -627,7 +629,7 @@ resource "github_organization_ruleset" "test" { creation = true } } - `, randomID) + `, resourceName, randomID) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasPaidOrgs(t) }, From dbccc72d94bb2c59285c9ee5149d6255355e67ea Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 7 Dec 2025 02:26:23 +0200 Subject: [PATCH 15/56] Fix linter issues 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 3204ca50d1..ce68da1dd1 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -868,7 +868,7 @@ 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 } From 6b2597639743c420f94899e95edf9056af88afd6 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 8 Dec 2025 15:40:59 +0200 Subject: [PATCH 16/56] 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 ce68da1dd1..aaeb6c89d8 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -282,9 +282,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 f03d9a4a74..43d3822e76 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -604,6 +604,56 @@ resource "github_organization_ruleset" "test" { }, }) }) + 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) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) From 65aa87b5706e8db18c5569a9de3cfa6b07207e6e Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 8 Dec 2025 15:59:19 +0200 Subject: [PATCH 17/56] Add test to ensure that `required_checks` is always required Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 87 +++++++++++++++---- 1 file changed, 70 insertions(+), 17 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 43d3822e76..0555890bc7 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -604,9 +604,12 @@ resource "github_organization_ruleset" "test" { }, }) }) - 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" + randomID := acctest.RandString(5) + config := fmt.Sprintf(` resource "github_organization_ruleset" "%s" { name = "test-context-is-not-empty-%s" target = "branch" @@ -633,25 +636,75 @@ resource "github_organization_ruleset" "test" { } `, 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 testMode) { + 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" + randomID := acctest.RandString(5) + config := fmt.Sprintf(` + resource "github_organization_ruleset" "%s" { + name = "test-required-with-%s" + target = "branch" + enforcement = "active" + + conditions { + ref_name { + include = ["~ALL"] + exclude = [] + } + repository_name { + include = ["~ALL"] + exclude = [] + } + } - 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.") + rules { + required_status_checks { + strict_required_status_checks_policy = true + } + } + } + `, resourceName, randomID) + + testCase := func(t *testing.T, mode testMode) { + 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 7319af88ea0d6ce3e4af3bcd86a475a188825426 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 10 Dec 2025 21:19:00 +0200 Subject: [PATCH 18/56] Fix tests after rebase Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 58 ++++++------------- 1 file changed, 19 insertions(+), 39 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 0555890bc7..bcc6c62569 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -605,8 +605,8 @@ resource "github_organization_ruleset" "test" { }) }) - t.Run("Validates rules.required_status_checks", func(t *testing.T) { - t.Run("required_check.context should not be empty", func(t *testing.T) { + t.Run("validates_rules__required_status_checks_block", func(t *testing.T) { + t.Run("required_check__context_block_should_not_be_empty", func(t *testing.T) { resourceName := "test-required-status-checks-context-is-not-empty" randomID := acctest.RandString(5) config := fmt.Sprintf(` @@ -636,28 +636,18 @@ resource "github_organization_ruleset" "test" { } `, resourceName, randomID) - testCase := func(t *testing.T, mode testMode) { - 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"), - }, + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, + 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("required_check should be required when strict_required_status_checks_policy is set", func(t *testing.T) { + 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" randomID := acctest.RandString(5) config := fmt.Sprintf(` @@ -685,25 +675,15 @@ resource "github_organization_ruleset" "test" { } `, resourceName, randomID) - testCase := func(t *testing.T, mode testMode) { - resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessMode(t, mode) }, - Providers: testAccProviders, - Steps: []resource.TestStep{ - { - Config: config, - ExpectError: regexp.MustCompile("Insufficient required_check blocks"), - }, + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, + 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 e585959dc7f4d3973f74000a42539f9e80ba94c0 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 10 Dec 2025 21:54:37 +0200 Subject: [PATCH 19/56] 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 aaeb6c89d8..a491cfc6f3 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -90,7 +90,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. `ref_name` is required for `branch` and `tag` targets, but must not be set for `push` or `repository` targets. One of `repository_name` or `repository_id` is always required.", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "ref_name": { From 7bf84e0685484a5a679d9eb0183f794548aac20b Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 14 Dec 2025 09:07:47 +0200 Subject: [PATCH 20/56] 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 a491cfc6f3..2c7a60fe3b 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -46,7 +46,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 @@ -65,7 +65,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, @@ -94,9 +94,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": { @@ -122,6 +123,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{ @@ -313,7 +315,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, @@ -617,8 +619,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 6190529f0b708ea7a7a3cd9cb58a780699014264 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 14 Dec 2025 19:50:33 +0200 Subject: [PATCH 21/56] 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 | 66 ++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index bcc6c62569..180be710b8 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -588,7 +588,10 @@ resource "github_organization_ruleset" "test" { } rules { - creation = true + # Push rulesets only support push-specific rules + max_file_size { + max_file_size = 100 + } } } `, resourceName, randomID) @@ -605,6 +608,67 @@ resource "github_organization_ruleset" "test" { }) }) + t.Run("creates_push_ruleset_with_only_repository_name", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + 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", + ), + ) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + Check: check, + }, + }, + }) + }) + t.Run("validates_rules__required_status_checks_block", func(t *testing.T) { t.Run("required_check__context_block_should_not_be_empty", func(t *testing.T) { resourceName := "test-required-status-checks-context-is-not-empty" From 6e29a23c1ee76286f4a991933475918150ef7e44 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sun, 14 Dec 2025 19:54:08 +0200 Subject: [PATCH 22/56] Add failing test for `flattenConditions` with no `ref_name` condition Signed-off-by: Timo Sand --- github/util_rules_test.go | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/github/util_rules_test.go b/github/util_rules_test.go index 8aaec0aed4..32e30cdecc 100644 --- a/github/util_rules_test.go +++ b/github/util_rules_test.go @@ -473,3 +473,49 @@ func TestCopilotCodeReviewRoundTrip(t *testing.T) { t.Errorf("Expected review_draft_pull_requests to be false, got %v", copilotRules[0]["review_draft_pull_requests"]) } } + +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 1bd4c6dee79a7fd2bfb6032708edfeb114043876 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 15 Dec 2025 06:58:20 +0200 Subject: [PATCH 23/56] Fix `flattenConditions` to work with `push` rulesets Signed-off-by: Timo Sand --- github/util_rules.go | 14 ++++++++------ github/util_rules_test.go | 9 +++------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/github/util_rules.go b/github/util_rules.go index 1c6c661656..ff59d492b1 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -218,19 +218,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/util_rules_test.go b/github/util_rules_test.go index 32e30cdecc..3c2d3e0ce1 100644 --- a/github/util_rules_test.go +++ b/github/util_rules_test.go @@ -494,12 +494,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 47d34a6d9fdec77a5dc7371638e41e29302430e3 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 15 Dec 2025 07:01:56 +0200 Subject: [PATCH 24/56] Add more tests for `flattenConditions` Signed-off-by: Timo Sand --- github/util_rules_test.go | 109 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/github/util_rules_test.go b/github/util_rules_test.go index 3c2d3e0ce1..d72cf76ad3 100644 --- a/github/util_rules_test.go +++ b/github/util_rules_test.go @@ -516,3 +516,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 b4e2626a5958c67f3ce04f8475aa3f2ae549de7f Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 15 Dec 2025 21:10:46 +0200 Subject: [PATCH 25/56] Enable debug logging in `flattenConditions` Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 2 +- github/util_rules.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 2c7a60fe3b..2ff2a79c4c 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -720,7 +720,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("etag", resp.Header.Get("ETag")) diff --git a/github/util_rules.go b/github/util_rules.go index ff59d492b1..fc02a82125 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -1,11 +1,13 @@ package github import ( + "context" "log" "reflect" "sort" "github.com/google/go-github/v82/github" + "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -218,7 +220,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 2da9d9758ac0801f0b284aa823c6b74ea28f4583 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Mon, 15 Dec 2025 21:18:22 +0200 Subject: [PATCH 26/56] Ensures that `flattenConditions` returns an empty list on empty API response Signed-off-by: Timo Sand --- github/util_rules.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/util_rules.go b/github/util_rules.go index fc02a82125..276b6eb895 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -224,8 +224,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 f330e2eae755a3882dbd43cc72c6a2df2f108d80 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 16 Dec 2025 19:35:59 +0200 Subject: [PATCH 27/56] 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 | 78 ++++++++++- 2 files changed, 203 insertions(+), 2 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 2ff2a79c4c..ac91ac48c3 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -11,6 +11,7 @@ import ( "github.com/google/go-github/v82/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" ) @@ -27,7 +28,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { SchemaVersion: 1, - CustomizeDiff: validateConditionsFieldBasedOnTarget, + CustomizeDiff: customdiff.All( + validateConditionsFieldBasedOnTarget, + validateRulesFieldBasedOnTarget, + ), Schema: map[string]*schema.Schema{ "name": { @@ -922,3 +926,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 180be710b8..b95faef27e 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -567,7 +567,7 @@ resource "github_organization_ruleset" "test" { }) }) - t.Run("validates_push_target_rejects_ref_name", func(t *testing.T) { + t.Run("validates_push_target_rejects_ref_name_condition", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) resourceName := "test-push-reject-ref-name" config := fmt.Sprintf(` @@ -608,6 +608,82 @@ resource "github_organization_ruleset" "test" { }) }) + t.Run("validates_push_target_rejects_branch_or_tag_rules", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + 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) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("rule .* is not valid for push target"), + }, + }, + }) + }) + + t.Run("validates_branch_target_rejects_push-only_rules", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + 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) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("rule .* is only valid for push target"), + }, + }, + }) + }) + t.Run("creates_push_ruleset_with_only_repository_name", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) resourceName := "test-push-repo-name-only" From a814af74e4a4aade9523300f91d79ca66fde0e21 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 17 Dec 2025 16:26:15 +0200 Subject: [PATCH 28/56] Get `TestAccGithubRepositoryRulesets` to work Signed-off-by: Timo Sand --- ...resource_github_repository_ruleset_test.go | 111 ++++++++---------- 1 file changed, 48 insertions(+), 63 deletions(-) diff --git a/github/resource_github_repository_ruleset_test.go b/github/resource_github_repository_ruleset_test.go index ddfb300852..f5936e4ab5 100644 --- a/github/resource_github_repository_ruleset_test.go +++ b/github/resource_github_repository_ruleset_test.go @@ -13,9 +13,11 @@ import ( ) func TestAccGithubRepositoryRuleset(t *testing.T) { - baseVisibility := "public" + baseRepoVisibility := "public" + if testAccConf.authMode == enterprise { - baseVisibility = "private" // Enable tests to run on GHEC EMU + // This enables repos to be created even in GHEC EMU + baseRepoVisibility = "private" } t.Run("create_branch_ruleset", func(t *testing.T) { @@ -90,34 +92,36 @@ resource "github_repository_ruleset" "test" { required_signatures = false pull_request { - dismiss_stale_reviews_on_push = true - require_code_owner_review = true - require_last_push_approval = true + allowed_merge_methods = ["merge", "squash"] required_approving_review_count = 2 required_review_thread_resolution = true + require_code_owner_review = true + dismiss_stale_reviews_on_push = true + require_last_push_approval = true } required_status_checks { - do_not_enforce_on_create = true - strict_required_status_checks_policy = true required_check { context = "ci" } - } - non_fast_forward = true + strict_required_status_checks_policy = true + do_not_enforce_on_create = true + } required_code_scanning { required_code_scanning_tool { - alerts_threshold = "errors" - security_alerts_threshold = "high_or_higher" - tool = "CodeQL" + alerts_threshold = "errors" + security_alerts_threshold = "high_or_higher" + tool = "CodeQL" } } + + non_fast_forward = true } } -`, repoName, baseVisibility) +`, repoName, baseRepoVisibility) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, @@ -186,10 +190,10 @@ resource "github_repository_ruleset" "test" { } } } -`, repoName, baseVisibility) +`, repoName, baseRepoVisibility) resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessMode(t, enterprise) }, + PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { @@ -277,8 +281,8 @@ resource "github_repository_ruleset" "test" { t.Run("update_ruleset_name", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) repoName := fmt.Sprintf("%srepo-ruleset-rename-%s", testResourcePrefix, randomID) - name := fmt.Sprintf(`ruleset-%[1]s`, randomID) - nameUpdated := fmt.Sprintf(`%[1]s-renamed`, randomID) + name := fmt.Sprintf("ruleset-%s", randomID) + nameUpdated := fmt.Sprintf("%s-renamed", name) config := ` resource "github_repository" "test" { @@ -305,13 +309,13 @@ resource "github_repository_ruleset" "test" { ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: fmt.Sprintf(config, repoName, randomID, baseVisibility, name), + Config: fmt.Sprintf(config, repoName, randomID, baseRepoVisibility, name), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("github_repository_ruleset.test", "name", name), ), }, { - Config: fmt.Sprintf(config, repoName, randomID, baseVisibility, nameUpdated), + Config: fmt.Sprintf(config, repoName, randomID, baseRepoVisibility, nameUpdated), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("github_repository_ruleset.test", "name", nameUpdated), ), @@ -324,45 +328,19 @@ resource "github_repository_ruleset" "test" { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) repoName := fmt.Sprintf("%srepo-ruleset-bypass-%s", testResourcePrefix, randomID) - config := fmt.Sprintf(` -resource "github_repository" "test" { - name = "%s" - description = "Terraform acceptance tests %[1]s" - auto_init = true - visibility = "%s" + bypassActorsConfig := ` +bypass_actors { + actor_type = "DeployKey" + bypass_mode = "always" } -resource "github_repository_ruleset" "test" { - name = "test-bypass" - repository = github_repository.test.id - target = "branch" - enforcement = "active" - - bypass_actors { - actor_type = "DeployKey" - bypass_mode = "always" - } - - bypass_actors { - actor_id = 5 - actor_type = "RepositoryRole" - bypass_mode = "always" - } - - conditions { - ref_name { - include = ["~ALL"] - exclude = [] - } - } - - rules { - creation = true - } +bypass_actors { + actor_id = 5 + actor_type = "RepositoryRole" + bypass_mode = "always" } -`, repoName, baseVisibility) - - configUpdated := fmt.Sprintf(` +` + baseConfig := ` resource "github_repository" "test" { name = "%s" description = "Terraform acceptance tests %[1]s" @@ -383,12 +361,16 @@ resource "github_repository_ruleset" "test" { } } + %s + rules { creation = true } } -`, repoName, baseVisibility) +` + config := fmt.Sprintf(baseConfig, repoName, baseRepoVisibility, bypassActorsConfig) + configUpdated := fmt.Sprintf(baseConfig, repoName, baseRepoVisibility, "") resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, @@ -454,13 +436,13 @@ resource "github_repository_ruleset" "test" { ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: fmt.Sprintf(config, repoName, randomID, baseVisibility, bypassMode), + Config: fmt.Sprintf(config, repoName, randomID, baseRepoVisibility, bypassMode), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("github_repository_ruleset.test", "bypass_actors.0.bypass_mode", bypassMode), ), }, { - Config: fmt.Sprintf(config, repoName, randomID, baseVisibility, bypassModeUpdated), + Config: fmt.Sprintf(config, repoName, randomID, baseRepoVisibility, bypassModeUpdated), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("github_repository_ruleset.test", "bypass_actors.0.bypass_mode", bypassModeUpdated), ), @@ -505,7 +487,7 @@ resource "github_repository_ruleset" "test" { creation = true } } - `, repoName, randomID, baseVisibility) + `, repoName, randomID, baseRepoVisibility) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, @@ -527,10 +509,13 @@ resource "github_repository_ruleset" "test" { } func TestAccGithubRepositoryRulesetArchived(t *testing.T) { - baseVisibility := "public" + baseRepoVisibility := "public" + if testAccConf.authMode == enterprise { - baseVisibility = "private" // Enable tests to run on GHEC EMU + // This enables repos to be created even in GHEC EMU + baseRepoVisibility = "private" } + t.Run("skips update and delete on archived repository", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) repoName := fmt.Sprintf("%srepo-ruleset-arch-%s", testResourcePrefix, randomID) @@ -549,7 +534,7 @@ func TestAccGithubRepositoryRulesetArchived(t *testing.T) { enforcement = "active" rules { creation = true } } - `, repoName, baseVisibility) + `, repoName, baseRepoVisibility) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, @@ -579,7 +564,7 @@ func TestAccGithubRepositoryRulesetArchived(t *testing.T) { enforcement = "active" rules { creation = true } } - `, repoName, baseVisibility) + `, repoName, baseRepoVisibility) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, From 6765136f96d706592ab2247887a6dc14eb4b115e Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 17 Dec 2025 20:59:49 +0200 Subject: [PATCH 29/56] `repository_ruleset`: Add tests for validations Signed-off-by: Timo Sand --- ...resource_github_repository_ruleset_test.go | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/github/resource_github_repository_ruleset_test.go b/github/resource_github_repository_ruleset_test.go index f5936e4ab5..a50e324df3 100644 --- a/github/resource_github_repository_ruleset_test.go +++ b/github/resource_github_repository_ruleset_test.go @@ -576,6 +576,177 @@ func TestAccGithubRepositoryRulesetArchived(t *testing.T) { }) } +func TestAccGithubRepositoryRulesetValidation(t *testing.T) { + t.Run("Validates push target rejects ref_name condition", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + + config := fmt.Sprintf(` + resource "github_repository" "test" { + name = "tf-acc-test-push-ref-%s" + auto_init = true + visibility = "private" + 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) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("ref_name must not be set for push target"), + }, + }, + }) + }) + + t.Run("Validates push target rejects branch/tag rules", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + config := fmt.Sprintf(` + resource "github_repository" "test" { + name = "tf-acc-test-push-rules-%s" + auto_init = true + visibility = "private" + 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) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("rule .* is not valid for push target"), + }, + }, + }) + }) + + t.Run("Validates branch target rejects push-only rules", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + 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) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("rule .* is only valid for push target"), + }, + }, + }) + }) + + t.Run("Validates tag target rejects push-only rules", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + 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) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasPaidOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile("rule .* is only valid for push target"), + }, + }, + }) + }) +} + func importRepositoryRulesetByResourcePaths(repoLogicalName, rulesetLogicalName string) resource.ImportStateIdFunc { // test importing using an ID of the form : return func(s *terraform.State) (string, error) { From d3278c079a14fd9207a7854c1ee8e435993ee223 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 17 Dec 2025 21:05:13 +0200 Subject: [PATCH 30/56] `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 182ae45c38..3492fac201 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -10,7 +10,9 @@ import ( "strconv" "github.com/google/go-github/v82/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" ) @@ -27,6 +29,11 @@ func resourceGithubRepositoryRuleset() *schema.Resource { SchemaVersion: 1, + CustomizeDiff: customdiff.All( + validateRepositoryRulesetConditions, + validateRepositoryRulesetRules, + ), + Schema: map[string]*schema.Schema{ "name": { Type: schema.TypeString, @@ -794,3 +801,84 @@ func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.Resour 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 134f52958fd71e76a99f3140b39eff1a412e08a3 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 17 Dec 2025 22:18:58 +0200 Subject: [PATCH 31/56] Updated ruleset docs Signed-off-by: Timo Sand --- .../docs/r/organization_ruleset.html.markdown | 25 +++++++++++-------- .../docs/r/repository_ruleset.html.markdown | 6 +++-- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/website/docs/r/organization_ruleset.html.markdown b/website/docs/r/organization_ruleset.html.markdown index 949b806637..ef9e803d9d 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](#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)) @@ -239,7 +240,7 @@ The `rules` block supports the following: - `do_not_enforce_on_create` - (Optional) (Boolean) Allow repositories and branches to be created if a check would otherwise prohibit it. Defaults to `false`. -- `required_workflow` - (Required) (Block Set, Min: 1) Actions workflows that are required. Multiple can be defined. (see [below for nested schema](#rulesrequired_workflows.required_workflow)) +- `required_workflow` - (Required) (Block Set, Min: 1) Actions workflows that are required. Multiple can be defined. (see [below for nested schema](#rulesrequired_workflowsrequired_workflow)) #### rules.required_workflows.required_workflow #### @@ -251,7 +252,7 @@ The `rules` block supports the following: #### rules.required_code_scanning #### -- `required_code_scanning_tool` - (Required) (Block Set, Min: 1) Actions code scanning tools that are required. Multiple can be defined. (see [below for nested schema](#rulesrequired_workflows.required_code_scanning_tool)) +- `required_code_scanning_tool` - (Required) (Block Set, Min: 1) Actions code scanning tools that are required. Multiple can be defined. (see [below for nested schema](#rulesrequired_code_scanningrequired_code_scanning_tool)) #### rules.required_code_scanning.required_code_scanning_tool #### @@ -305,12 +306,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](#conditionsref_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)) +- `repository_name` (Optional) (Block List, Max: 1) Conflicts with `repository_id`. (see [below for nested schema](#conditionsrepository_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 37893cecb9..264a716fc8 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)) @@ -294,7 +296,7 @@ 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` - (Required) (Block List, Min: 1, Max: 1) (see [below for nested schema](#conditionsref_name)) #### conditions.ref_name #### From 95f7182596be683cb4d89b2546d138c87a982956 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 31 Dec 2025 22:36:44 +0200 Subject: [PATCH 32/56] Extract validation functions to separate utils file with unit tests Signed-off-by: Timo Sand --- github/repository_rules_validation_utils.go | 171 +++++++++++++ .../repository_rules_validation_utils_test.go | 225 ++++++++++++++++++ .../resource_github_organization_ruleset.go | 164 +------------ ...source_github_organization_ruleset_test.go | 2 +- github/resource_github_repository_ruleset.go | 61 +---- ...resource_github_repository_ruleset_test.go | 4 +- 6 files changed, 418 insertions(+), 209 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..a60e34ae08 --- /dev/null +++ b/github/repository_rules_validation_utils.go @@ -0,0 +1,171 @@ +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 +} + +func validateConditionsFieldForRepositoryTarget(ctx context.Context, conditions map[string]any) error { + tflog.Debug(ctx, "Validating conditions field for repository target", map[string]any{"target": "repository", "conditions": conditions}) + + if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { + tflog.Debug(ctx, "Invalid ref_name for repository target", map[string]any{"ref_name": conditions["ref_name"]}) + return fmt.Errorf("ref_name must not be set for repository 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("one of repository_name or repository_id must be set for repository target") + } + tflog.Debug(ctx, "Conditions validation passed for repository 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 ac91ac48c3..67c4cff718 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -30,7 +30,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { CustomizeDiff: customdiff.All( validateConditionsFieldBasedOnTarget, - validateRulesFieldBasedOnTarget, + validateOrganizationRulesetRules, ), Schema: map[string]*schema.Schema{ @@ -868,44 +868,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 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}) @@ -916,134 +878,28 @@ 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 validateConditionsFieldForPushTarget(ctx, conditions) case "repository": - return validateConditionsFieldForRepositoryTarget(ctx, d, meta) + return validateConditionsFieldForRepositoryTarget(ctx, conditions) } 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 { +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_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index b95faef27e..d9e99219a0 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -678,7 +678,7 @@ resource "github_organization_ruleset" "test" { 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"), }, }, }) diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index 3492fac201..3fad759ffd 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -803,8 +803,6 @@ func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.Resour } // 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}) @@ -817,26 +815,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 { @@ -844,41 +834,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 a50e324df3..220a318f9d 100644 --- a/github/resource_github_repository_ruleset_test.go +++ b/github/resource_github_repository_ruleset_test.go @@ -695,7 +695,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"), }, }, }) @@ -740,7 +740,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 40d21febae372a225575807c3bb27156a612de1e Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 10 Jan 2026 10:56:10 +0200 Subject: [PATCH 33/56] Fix push ruleset test config Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index d9e99219a0..a9d79afb72 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -211,11 +211,6 @@ resource "github_organization_ruleset" "test" { include = ["~ALL"] exclude = [] } - - ref_name { - include = ["~ALL"] - exclude = [] - } } rules { From 6d2f5d5a28fcf68913147d120e51bc2389e73125 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 10 Jan 2026 13:21:56 +0200 Subject: [PATCH 34/56] Remove `repository` target after thorough testing that it doesn't do anything Signed-off-by: Timo Sand --- github/repository_rules_validation_utils.go | 14 ------ .../resource_github_organization_ruleset.go | 13 +++-- ...source_github_organization_ruleset_test.go | 50 +++---------------- 3 files changed, 12 insertions(+), 65 deletions(-) diff --git a/github/repository_rules_validation_utils.go b/github/repository_rules_validation_utils.go index a60e34ae08..a475e4dd28 100644 --- a/github/repository_rules_validation_utils.go +++ b/github/repository_rules_validation_utils.go @@ -155,17 +155,3 @@ func validateConditionsFieldForPushTarget(ctx context.Context, conditions map[st tflog.Debug(ctx, "Conditions validation passed for push target") return nil } - -func validateConditionsFieldForRepositoryTarget(ctx context.Context, conditions map[string]any) error { - tflog.Debug(ctx, "Validating conditions field for repository target", map[string]any{"target": "repository", "conditions": conditions}) - - if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { - tflog.Debug(ctx, "Invalid ref_name for repository target", map[string]any{"ref_name": conditions["ref_name"]}) - return fmt.Errorf("ref_name must not be set for repository 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("one of repository_name or repository_id must be set for repository target") - } - tflog.Debug(ctx, "Conditions validation passed for repository target") - return nil -} diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 67c4cff718..e8c8f559ec 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -41,10 +41,11 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "The name of the ruleset.", }, "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`.", + Type: schema.TypeString, + Required: true, + // The API accepts an `repository` target, but any rule created with that doesn't show up in the UI, nor does it have any rules. + 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, @@ -94,7 +95,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Type: schema.TypeList, Optional: true, MaxItems: 1, - Description: "Parameters for an organization ruleset condition. `ref_name` is required for `branch` and `tag` targets, but must not be set for `push` or `repository` targets. One of `repository_name` or `repository_id` is always required.", + 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": { @@ -885,8 +886,6 @@ func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.Resourc return validateConditionsFieldForBranchAndTagTargets(ctx, target, conditions) case "push": return validateConditionsFieldForPushTarget(ctx, conditions) - case "repository": - return validateConditionsFieldForRepositoryTarget(ctx, conditions) } return nil } diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index a9d79afb72..940cda0c38 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -35,10 +35,10 @@ name: Echo Workflow on: [pull_request] jobs: - echo: - runs-on: linux - steps: - - run: echo \"Hello, world!\" + echo: + runs-on: linux + steps: + - run: echo \"Hello, world!\" EOT commit_message = "Managed by Terraform" commit_author = "Terraform User" @@ -679,9 +679,9 @@ resource "github_organization_ruleset" "test" { }) }) - t.Run("creates_push_ruleset_with_only_repository_name", func(t *testing.T) { + t.Run("creates_push_ruleset", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) - resourceName := "test-push-repo-name-only" + resourceName := "test-push-ruleset" config := fmt.Sprintf(` resource "github_organization_ruleset" "%s" { name = "test-push-%s" @@ -822,44 +822,6 @@ resource "github_organization_ruleset" "test" { }) }) }) - - t.Run("validates_repository_target_rejects_ref_name", func(t *testing.T) { - randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) - 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) - - resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnlessHasPaidOrgs(t) }, - ProviderFactories: providerFactories, - Steps: []resource.TestStep{ - { - Config: config, - ExpectError: regexp.MustCompile("ref_name must not be set for repository target"), - }, - }, - }) - }) } func TestOrganizationPushRulesetSupport(t *testing.T) { From 6a7434f8fc1fa0986800ee5b4cb9b41e9e7e44fe Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 13 Jan 2026 22:14:45 +0200 Subject: [PATCH 35/56] Use idiomatic test naming convention Signed-off-by: Timo Sand --- github/repository_rules_validation_utils_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/github/repository_rules_validation_utils_test.go b/github/repository_rules_validation_utils_test.go index 91915561fb..63afd63705 100644 --- a/github/repository_rules_validation_utils_test.go +++ b/github/repository_rules_validation_utils_test.go @@ -4,7 +4,7 @@ import ( "testing" ) -func TestValidateConditionsFieldForPushTarget(t *testing.T) { +func Test_validateConditionsFieldForPushTarget(t *testing.T) { tests := []struct { name string conditions map[string]any @@ -56,7 +56,7 @@ func TestValidateConditionsFieldForPushTarget(t *testing.T) { } } -func TestValidateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *testing.T) { +func Test_validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *testing.T) { tests := []struct { name string target string @@ -128,7 +128,7 @@ func TestValidateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *testi } } -func TestValidateConditionsFieldForBranchAndTagTargets(t *testing.T) { +func Test_validateConditionsFieldForBranchAndTagTargets(t *testing.T) { tests := []struct { name string target string @@ -214,7 +214,7 @@ func TestValidateConditionsFieldForBranchAndTagTargets(t *testing.T) { } } -func TestRuleListsDoNotOverlap(t *testing.T) { +func Test_ruleListsDoNotOverlap(t *testing.T) { for _, pushRule := range pushOnlyRules { for _, branchTagRule := range branchTagOnlyRules { if pushRule == branchTagRule { From 64af81e61801495fec69a27d5b91aaf53d6843ea Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 13 Jan 2026 22:27:07 +0200 Subject: [PATCH 36/56] Address code structure comment in tests Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 36 ++++++------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 940cda0c38..d1ee7fb381 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -681,10 +681,12 @@ resource "github_organization_ruleset" "test" { t.Run("creates_push_ruleset", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + rulesetName := fmt.Sprintf("%stest-push-%s", testResourcePrefix, randomID) resourceName := "test-push-ruleset" + resourceFullName := fmt.Sprintf("github_organization_ruleset.%s", resourceName) config := fmt.Sprintf(` resource "github_organization_ruleset" "%s" { - name = "test-push-%s" + name = "%s" target = "push" enforcement = "active" @@ -703,30 +705,7 @@ resource "github_organization_ruleset" "test" { } } } - `, 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", - ), - ) + `, resourceName, rulesetName) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasPaidOrgs(t) }, @@ -734,7 +713,12 @@ resource "github_organization_ruleset" "test" { Steps: []resource.TestStep{ { Config: config, - Check: check, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceFullName, "name", rulesetName), + resource.TestCheckResourceAttr(resourceFullName, "target", "push"), + resource.TestCheckResourceAttr(resourceFullName, "enforcement", "active"), + resource.TestCheckResourceAttr(resourceFullName, "rules.0.max_file_size.0.max_file_size", "100"), + ), }, }, }) From fe652057a1f8331bc100d7b5e31e59356d94e501 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 13 Jan 2026 23:52:17 +0200 Subject: [PATCH 37/56] Fix inconsistent logging Signed-off-by: Timo Sand --- github/resource_github_repository_ruleset.go | 21 +++++++------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index 3fad759ffd..da7f7fbfd5 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -3,8 +3,6 @@ package github import ( "context" "errors" - "fmt" - "log" "net/http" "regexp" "strconv" @@ -690,8 +688,7 @@ func resourceGithubRepositoryRulesetRead(ctx context.Context, d *schema.Resource return nil } if ghErr.Response.StatusCode == http.StatusNotFound { - log.Printf("[INFO] Removing ruleset %s/%s: %d from state because it no longer exists in GitHub", - owner, repoName, rulesetID) + tflog.Info(ctx, "Removing ruleset from state because it no longer exists in GitHub", map[string]any{"owner": owner, "repo_name": repoName, "ruleset_id": rulesetID}) d.SetId("") return nil } @@ -700,8 +697,7 @@ func resourceGithubRepositoryRulesetRead(ctx context.Context, d *schema.Resource } if ruleset == nil { - log.Printf("[INFO] Removing ruleset %s/%s: %d from state because it no longer exists in GitHub (empty response)", - owner, repoName, rulesetID) + tflog.Info(ctx, "Removing ruleset from state because it no longer exists in GitHub (empty response)", map[string]any{"owner": owner, "repo_name": repoName, "ruleset_id": rulesetID}) d.SetId("") return nil } @@ -738,7 +734,7 @@ func resourceGithubRepositoryRulesetUpdate(ctx context.Context, d *schema.Resour return diag.FromErr(err) } if repo.GetArchived() { - log.Printf("[INFO] Repository %s/%s is archived, skipping ruleset update", owner, repoName) + tflog.Info(ctx, "Repository is archived, skipping ruleset update", map[string]any{"owner": owner, "repo_name": repoName}) return nil } @@ -764,9 +760,9 @@ func resourceGithubRepositoryRulesetDelete(ctx context.Context, d *schema.Resour return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } - log.Printf("[DEBUG] Deleting repository ruleset: %s/%s: %d", owner, repoName, rulesetID) + tflog.Debug(ctx, "Deleting repository ruleset", map[string]any{"owner": owner, "repo_name": repoName, "ruleset_id": rulesetID}) _, err = client.Repositories.DeleteRuleset(ctx, owner, repoName, rulesetID) - return diag.FromErr(handleArchivedRepoDelete(err, "repository ruleset", fmt.Sprintf("%d", rulesetID), owner, repoName)) + return diag.FromErr(handleArchivedRepoDelete(err, "repository ruleset", strconv.FormatInt(rulesetID, 10), owner, repoName)) } func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { @@ -782,11 +778,11 @@ func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.Resour if rulesetID == 0 { return []*schema.ResourceData{d}, fmt.Errorf("`ruleset_id` must be present") } - log.Printf("[DEBUG] Importing repository ruleset with ID: %d, for repository: %s", rulesetID, repoName) - client := meta.(*Owner).v3client owner := meta.(*Owner).name + tflog.Debug(ctx, "Importing repository ruleset", map[string]any{"owner": owner, "repo_name": repoName, "ruleset_id": rulesetID}) + repository, _, err := client.Repositories.Get(ctx, owner, repoName) if repository == nil || err != nil { return []*schema.ResourceData{d}, err @@ -834,8 +830,5 @@ func validateRepositoryRulesetRules(ctx context.Context, d *schema.ResourceDiff, return nil } - value, exists := d.GetOk("rules.0.update_allows_fetch_and_merge") - tflog.Debug(ctx, "FOO", map[string]any{"foo": value, "exists": exists}) - return validateRulesForTarget(ctx, d) } From 2a39e8224ed83daf8bfe4a4a309700ec8f9c8fe4 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 14 Jan 2026 00:26:37 +0200 Subject: [PATCH 38/56] Refactor to use typed constant string for ruleset `Target` Signed-off-by: Timo Sand --- github/repository_rules_validation_utils.go | 14 +++++---- .../repository_rules_validation_utils_test.go | 30 ++++++++++--------- .../resource_github_organization_ruleset.go | 10 +++---- github/resource_github_repository_ruleset.go | 11 +++---- ...resource_github_repository_ruleset_test.go | 19 +++++++----- 5 files changed, 46 insertions(+), 38 deletions(-) diff --git a/github/repository_rules_validation_utils.go b/github/repository_rules_validation_utils.go index a475e4dd28..934b769ada 100644 --- a/github/repository_rules_validation_utils.go +++ b/github/repository_rules_validation_utils.go @@ -5,6 +5,7 @@ import ( "fmt" "slices" + "github.com/google/go-github/v82/github" "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -40,6 +41,7 @@ var branchTagOnlyRules = []string{ "required_code_scanning", "required_deployments", "merge_queue", + "copilot_code_review", } // pushOnlyRules contains rules that are only valid for push targets. @@ -62,13 +64,13 @@ var pushOnlyRules = []string{ } func validateRulesForTarget(ctx context.Context, d *schema.ResourceDiff) error { - target := d.Get("target").(string) + target := github.RulesetTarget(d.Get("target").(string)) tflog.Debug(ctx, "Validating rules for target", map[string]any{"target": target}) switch target { - case "push": + case github.RulesetTargetPush: return validateRulesForPushTarget(ctx, d) - case "branch", "tag": + case github.RulesetTargetBranch, github.RulesetTargetTag: return validateRulesForBranchTagTarget(ctx, d) } @@ -85,7 +87,7 @@ func validateRulesForBranchTagTarget(ctx context.Context, d *schema.ResourceDiff } func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []string) error { - target := d.Get("target").(string) + target := github.RulesetTarget(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)) @@ -117,7 +119,7 @@ func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []s return nil } -func validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx context.Context, target string, conditions map[string]any) error { +func validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx context.Context, target github.RulesetTarget, 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 { @@ -129,7 +131,7 @@ func validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx context. return nil } -func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, target string, conditions map[string]any) error { +func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, target github.RulesetTarget, 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 { diff --git a/github/repository_rules_validation_utils_test.go b/github/repository_rules_validation_utils_test.go index 63afd63705..afa8e19515 100644 --- a/github/repository_rules_validation_utils_test.go +++ b/github/repository_rules_validation_utils_test.go @@ -2,6 +2,8 @@ package github import ( "testing" + + "github.com/google/go-github/v82/github" ) func Test_validateConditionsFieldForPushTarget(t *testing.T) { @@ -59,14 +61,14 @@ func Test_validateConditionsFieldForPushTarget(t *testing.T) { func Test_validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *testing.T) { tests := []struct { name string - target string + target github.RulesetTarget conditions map[string]any expectError bool errorMsg string }{ { name: "valid branch target with ref_name", - target: "branch", + target: github.RulesetTargetBranch, conditions: map[string]any{ "ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}}, }, @@ -74,7 +76,7 @@ func Test_validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *test }, { name: "valid tag target with ref_name", - target: "tag", + target: github.RulesetTargetTag, conditions: map[string]any{ "ref_name": []any{map[string]any{"include": []any{"v*"}, "exclude": []any{}}}, }, @@ -82,28 +84,28 @@ func Test_validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *test }, { name: "invalid branch target without ref_name", - target: "branch", + target: github.RulesetTargetBranch, conditions: map[string]any{}, expectError: true, errorMsg: "ref_name must be set for branch target", }, { name: "invalid tag target without ref_name", - target: "tag", + target: github.RulesetTargetTag, 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", + target: github.RulesetTargetBranch, 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", + target: github.RulesetTargetTag, conditions: map[string]any{"ref_name": []any{}}, expectError: true, errorMsg: "ref_name must be set for tag target", @@ -131,14 +133,14 @@ func Test_validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *test func Test_validateConditionsFieldForBranchAndTagTargets(t *testing.T) { tests := []struct { name string - target string + target github.RulesetTarget conditions map[string]any expectError bool errorMsg string }{ { name: "valid branch target with ref_name and repository_name", - target: "branch", + target: github.RulesetTargetBranch, 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{}}}, @@ -147,7 +149,7 @@ func Test_validateConditionsFieldForBranchAndTagTargets(t *testing.T) { }, { name: "valid tag target with ref_name and repository_id", - target: "tag", + target: github.RulesetTargetTag, conditions: map[string]any{ "ref_name": []any{map[string]any{"include": []any{"v*"}, "exclude": []any{}}}, "repository_id": []any{123, 456}, @@ -156,7 +158,7 @@ func Test_validateConditionsFieldForBranchAndTagTargets(t *testing.T) { }, { name: "invalid branch target without ref_name", - target: "branch", + target: github.RulesetTargetBranch, conditions: map[string]any{ "repository_name": []any{map[string]any{"include": []any{"~ALL"}, "exclude": []any{}}}, }, @@ -165,7 +167,7 @@ func Test_validateConditionsFieldForBranchAndTagTargets(t *testing.T) { }, { name: "invalid branch target without repository_name or repository_id", - target: "branch", + target: github.RulesetTargetBranch, conditions: map[string]any{ "ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}}, }, @@ -174,7 +176,7 @@ func Test_validateConditionsFieldForBranchAndTagTargets(t *testing.T) { }, { name: "invalid tag target with nil repository_name and repository_id", - target: "tag", + target: github.RulesetTargetTag, conditions: map[string]any{ "ref_name": []any{map[string]any{"include": []any{"v*"}, "exclude": []any{}}}, "repository_name": nil, @@ -185,7 +187,7 @@ func Test_validateConditionsFieldForBranchAndTagTargets(t *testing.T) { }, { name: "invalid branch target with empty repository_name and repository_id slices", - target: "branch", + target: github.RulesetTargetBranch, conditions: map[string]any{ "ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}}, "repository_name": []any{}, diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index e8c8f559ec..e4db446f32 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -44,7 +44,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Type: schema.TypeString, Required: true, // The API accepts an `repository` target, but any rule created with that doesn't show up in the UI, nor does it have any rules. - ValidateFunc: validation.StringInSlice([]string{"branch", "tag", "push"}, false), + ValidateFunc: validation.StringInSlice([]string{string(github.RulesetTargetBranch), string(github.RulesetTargetTag), string(github.RulesetTargetPush)}, false), Description: "The target of the ruleset. Possible values are `branch`, `tag` and `push`.", }, "enforcement": { @@ -870,7 +870,7 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso } func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { - target := d.Get("target").(string) + target := github.RulesetTarget(d.Get("target").(string)) tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target}) conditionsRaw := d.Get("conditions").([]any) @@ -882,16 +882,16 @@ func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.Resourc conditions := conditionsRaw[0].(map[string]any) switch target { - case "branch", "tag": + case github.RulesetTargetBranch, github.RulesetTargetTag: return validateConditionsFieldForBranchAndTagTargets(ctx, target, conditions) - case "push": + case github.RulesetTargetPush: return validateConditionsFieldForPushTarget(ctx, conditions) } return nil } func validateOrganizationRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error { - target := d.Get("target").(string) + target := github.RulesetTarget(d.Get("target").(string)) tflog.Debug(ctx, "Validating organization ruleset rules based on target", map[string]any{"target": target}) rulesRaw := d.Get("rules").([]any) diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index da7f7fbfd5..9d6f2132be 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -3,6 +3,7 @@ package github import ( "context" "errors" + "fmt" "net/http" "regexp" "strconv" @@ -42,7 +43,7 @@ func resourceGithubRepositoryRuleset() *schema.Resource { "target": { Type: schema.TypeString, Required: true, - ValidateFunc: validation.StringInSlice([]string{"branch", "push", "tag"}, false), + ValidateFunc: validation.StringInSlice([]string{string(github.RulesetTargetBranch), string(github.RulesetTargetPush), string(github.RulesetTargetTag)}, false), Description: "Possible values are `branch`, `push` and `tag`.", }, "repository": { @@ -800,7 +801,7 @@ func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.Resour // validateRepositoryRulesetConditions validates conditions based on target type. func validateRepositoryRulesetConditions(ctx context.Context, d *schema.ResourceDiff, _ any) error { - target := d.Get("target").(string) + target := github.RulesetTarget(d.Get("target").(string)) tflog.Debug(ctx, "Validating repository ruleset conditions", map[string]any{"target": target}) conditionsRaw := d.Get("conditions").([]any) @@ -812,16 +813,16 @@ func validateRepositoryRulesetConditions(ctx context.Context, d *schema.Resource conditions := conditionsRaw[0].(map[string]any) switch target { - case "branch", "tag": + case github.RulesetTargetBranch, github.RulesetTargetTag: return validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx, target, conditions) - case "push": + case github.RulesetTargetPush: return validateConditionsFieldForPushTarget(ctx, conditions) } return nil } func validateRepositoryRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error { - target := d.Get("target").(string) + target := github.RulesetTarget(d.Get("target").(string)) tflog.Debug(ctx, "Validating repository ruleset rules based on target", map[string]any{"target": target}) rulesRaw := d.Get("rules").([]any) diff --git a/github/resource_github_repository_ruleset_test.go b/github/resource_github_repository_ruleset_test.go index 220a318f9d..5af5a2f86a 100644 --- a/github/resource_github_repository_ruleset_test.go +++ b/github/resource_github_repository_ruleset_test.go @@ -4,7 +4,6 @@ import ( "fmt" "log" "regexp" - "strings" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" @@ -519,11 +518,15 @@ func TestAccGithubRepositoryRulesetArchived(t *testing.T) { t.Run("skips update and delete on archived repository", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) repoName := fmt.Sprintf("%srepo-ruleset-arch-%s", testResourcePrefix, randomID) - config := fmt.Sprintf(` + archivedBefore := false + archivedAfter := true + enforcementBefore := "active" + enforcementAfter := "disabled" + config := ` resource "github_repository" "test" { name = "%s" auto_init = true - archived = false + archived = %t visibility = "%s" } @@ -531,18 +534,18 @@ func TestAccGithubRepositoryRulesetArchived(t *testing.T) { name = "test" repository = github_repository.test.name target = "branch" - enforcement = "active" + enforcement = "%s" rules { creation = true } } - `, repoName, baseRepoVisibility) + ` resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ - {Config: config}, - {Config: strings.Replace(config, "archived = false", "archived = true", 1)}, - {Config: strings.Replace(strings.Replace(config, "archived = false", "archived = true", 1), `enforcement = "active"`, `enforcement = "disabled"`, 1)}, + {Config: fmt.Sprintf(config, repoName, archivedBefore, baseRepoVisibility, enforcementBefore)}, + {Config: fmt.Sprintf(config, repoName, archivedAfter, baseRepoVisibility, enforcementBefore)}, + {Config: fmt.Sprintf(config, repoName, archivedAfter, baseRepoVisibility, enforcementAfter)}, }, }) }) From d60c2e02fec703ec6dc946e86d9085c2a8e29c8a Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 14 Jan 2026 22:07:31 +0200 Subject: [PATCH 39/56] Fix indentation issue with `github_repository_file` and heredocs Signed-off-by: Timo Sand --- ...source_github_organization_ruleset_test.go | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index d1ee7fb381..ba3045cd3a 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -19,6 +19,19 @@ func TestAccGithubOrganizationRuleset(t *testing.T) { workflowFilePath := ".github/workflows/echo.yaml" config := fmt.Sprintf(` +locals { + workflow_content = < Date: Wed, 14 Jan 2026 23:19:52 +0200 Subject: [PATCH 40/56] Rename files to be more sensible Signed-off-by: Timo Sand --- ...itory_rules_validation_utils.go => util_ruleset_validation.go} | 0 ...s_validation_utils_test.go => util_ruleset_validation_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename github/{repository_rules_validation_utils.go => util_ruleset_validation.go} (100%) rename github/{repository_rules_validation_utils_test.go => util_ruleset_validation_test.go} (100%) diff --git a/github/repository_rules_validation_utils.go b/github/util_ruleset_validation.go similarity index 100% rename from github/repository_rules_validation_utils.go rename to github/util_ruleset_validation.go diff --git a/github/repository_rules_validation_utils_test.go b/github/util_ruleset_validation_test.go similarity index 100% rename from github/repository_rules_validation_utils_test.go rename to github/util_ruleset_validation_test.go From 4a0e1a340bc1cb916fd603564eae9babc1ab9ee6 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Thu, 15 Jan 2026 01:58:46 +0200 Subject: [PATCH 41/56] Refactor validation functions so that Repo and Org share almost everything Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 42 ++++------------- github/resource_github_repository_ruleset.go | 43 ++++------------- github/util_ruleset_validation.go | 47 ++++++++++++++----- github/util_ruleset_validation_test.go | 4 +- 4 files changed, 56 insertions(+), 80 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index e4db446f32..d76f8b5c67 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -11,7 +11,6 @@ import ( "github.com/google/go-github/v82/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,10 +27,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { SchemaVersion: 1, - CustomizeDiff: customdiff.All( - validateConditionsFieldBasedOnTarget, - validateOrganizationRulesetRules, - ), + CustomizeDiff: resourceGithubOrganizationRulesetValidate, Schema: map[string]*schema.Schema{ "name": { @@ -869,36 +865,16 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso return []*schema.ResourceData{d}, nil } -func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { - target := github.RulesetTarget(d.Get("target").(string)) - tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target}) - conditionsRaw := d.Get("conditions").([]any) - - if len(conditionsRaw) == 0 { - tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]any{"target": target}) - return nil - } - - conditions := conditionsRaw[0].(map[string]any) - - switch target { - case github.RulesetTargetBranch, github.RulesetTargetTag: - return validateConditionsFieldForBranchAndTagTargets(ctx, target, conditions) - case github.RulesetTargetPush: - return validateConditionsFieldForPushTarget(ctx, conditions) +func resourceGithubOrganizationRulesetValidate(ctx context.Context, d *schema.ResourceDiff, _ any) error { + err := validateRulesetConditions(ctx, d, true) + if err != nil { + return err } - return nil -} - -func validateOrganizationRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error { - target := github.RulesetTarget(d.Get("target").(string)) - 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 + err = validateRulesetRules(ctx, d) + if err != nil { + return err } - return validateRulesForTarget(ctx, d) + return nil } diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index 9d6f2132be..e25f87a117 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -11,7 +11,6 @@ import ( "github.com/google/go-github/v82/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,10 +27,7 @@ func resourceGithubRepositoryRuleset() *schema.Resource { SchemaVersion: 1, - CustomizeDiff: customdiff.All( - validateRepositoryRulesetConditions, - validateRepositoryRulesetRules, - ), + CustomizeDiff: resourceGithubRepositoryRulesetValidate, Schema: map[string]*schema.Schema{ "name": { @@ -799,37 +795,16 @@ func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.Resour return []*schema.ResourceData{d}, nil } -// validateRepositoryRulesetConditions validates conditions based on target type. -func validateRepositoryRulesetConditions(ctx context.Context, d *schema.ResourceDiff, _ any) error { - target := github.RulesetTarget(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) - - switch target { - case github.RulesetTargetBranch, github.RulesetTargetTag: - return validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx, target, conditions) - case github.RulesetTargetPush: - return validateConditionsFieldForPushTarget(ctx, conditions) +func resourceGithubRepositoryRulesetValidate(ctx context.Context, d *schema.ResourceDiff, meta any) error { + err := validateRulesetConditions(ctx, d, false) + if err != nil { + return err } - return nil -} - -func validateRepositoryRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error { - target := github.RulesetTarget(d.Get("target").(string)) - tflog.Debug(ctx, "Validating repository 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 + err = validateRulesetRules(ctx, d) + if err != nil { + return err } - return validateRulesForTarget(ctx, d) + return nil } diff --git a/github/util_ruleset_validation.go b/github/util_ruleset_validation.go index 934b769ada..686e3ee0c2 100644 --- a/github/util_ruleset_validation.go +++ b/github/util_ruleset_validation.go @@ -119,29 +119,54 @@ func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []s return nil } -func validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx context.Context, target github.RulesetTarget, conditions map[string]any) error { - tflog.Debug(ctx, fmt.Sprintf("Validating conditions field for %s target", target), map[string]any{"target": target, "conditions": conditions}) +func validateRulesetConditions(ctx context.Context, d *schema.ResourceDiff, isOrg bool) error { + target := github.RulesetTarget(d.Get("target").(string)) + tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target}) + conditionsRaw := d.Get("conditions").([]any) - 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 len(conditionsRaw) == 0 { + tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]any{"target": target}) + return nil } - tflog.Debug(ctx, fmt.Sprintf("Conditions validation passed for %s target", target)) + conditions := conditionsRaw[0].(map[string]any) + + switch target { + case github.RulesetTargetBranch, github.RulesetTargetTag: + return validateConditionsFieldForBranchAndTagTargets(ctx, target, conditions, isOrg) + case github.RulesetTargetPush: + return validateConditionsFieldForPushTarget(ctx, conditions) + } return nil } -func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, target github.RulesetTarget, conditions map[string]any) error { - tflog.Debug(ctx, fmt.Sprintf("Validating conditions field for %s target", target), map[string]any{"target": target, "conditions": conditions}) +func validateRulesetRules(ctx context.Context, d *schema.ResourceDiff) error { + target := github.RulesetTarget(d.Get("target").(string)) + tflog.Debug(ctx, "Validating 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 + } + + return validateRulesForTarget(ctx, d) +} + +func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, target github.RulesetTarget, conditions map[string]any, isOrg bool) error { + tflog.Debug(ctx, fmt.Sprintf("Validating conditions field for %s target", target), map[string]any{"target": target, "conditions": conditions, "isOrg": isOrg}) 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) + // Repository rulesets don't have repository_name or repository_id, only org rulesets do. + if isOrg { + 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 diff --git a/github/util_ruleset_validation_test.go b/github/util_ruleset_validation_test.go index afa8e19515..c3125fa6a5 100644 --- a/github/util_ruleset_validation_test.go +++ b/github/util_ruleset_validation_test.go @@ -114,7 +114,7 @@ func Test_validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *test for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions) + err := validateConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions, false) if tt.expectError { if err == nil { t.Errorf("expected error but got nil") @@ -200,7 +200,7 @@ func Test_validateConditionsFieldForBranchAndTagTargets(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions) + err := validateConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions, true) if tt.expectError { if err == nil { t.Errorf("expected error but got nil") From 946d0feedc9564c5b30cb086f3c0eb2cf020ddcd Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 21 Jan 2026 21:26:23 +0200 Subject: [PATCH 42/56] Address review comments Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 6 +++--- github/resource_github_repository_ruleset.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index d76f8b5c67..3ac780f008 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -27,7 +27,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { SchemaVersion: 1, - CustomizeDiff: resourceGithubOrganizationRulesetValidate, + CustomizeDiff: resourceGithubOrganizationRulesetDiff, Schema: map[string]*schema.Schema{ "name": { @@ -39,7 +39,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { "target": { Type: schema.TypeString, Required: true, - // The API accepts an `repository` target, but any rule created with that doesn't show up in the UI, nor does it have any rules. + // The API accepts an `repository` target, but we don't support it yet. ValidateFunc: validation.StringInSlice([]string{string(github.RulesetTargetBranch), string(github.RulesetTargetTag), string(github.RulesetTargetPush)}, false), Description: "The target of the ruleset. Possible values are `branch`, `tag` and `push`.", }, @@ -865,7 +865,7 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso return []*schema.ResourceData{d}, nil } -func resourceGithubOrganizationRulesetValidate(ctx context.Context, d *schema.ResourceDiff, _ any) error { +func resourceGithubOrganizationRulesetDiff(ctx context.Context, d *schema.ResourceDiff, _ any) error { err := validateRulesetConditions(ctx, d, true) if err != nil { return err diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index e25f87a117..f6f3389995 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -27,7 +27,7 @@ func resourceGithubRepositoryRuleset() *schema.Resource { SchemaVersion: 1, - CustomizeDiff: resourceGithubRepositoryRulesetValidate, + CustomizeDiff: resourceGithubRepositoryRulesetDiff, Schema: map[string]*schema.Schema{ "name": { @@ -795,7 +795,7 @@ func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.Resour return []*schema.ResourceData{d}, nil } -func resourceGithubRepositoryRulesetValidate(ctx context.Context, d *schema.ResourceDiff, meta any) error { +func resourceGithubRepositoryRulesetDiff(ctx context.Context, d *schema.ResourceDiff, meta any) error { err := validateRulesetConditions(ctx, d, false) if err != nil { return err From 155f642fb779854d5fee3a5327f9a261e1664387 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 00:05:23 +0200 Subject: [PATCH 43/56] Use `github.RepositoryRuleType` instead of strings Signed-off-by: Timo Sand --- github/util_ruleset_validation.go | 52 +++++++++++++++---------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/github/util_ruleset_validation.go b/github/util_ruleset_validation.go index 686e3ee0c2..e8e6e74371 100644 --- a/github/util_ruleset_validation.go +++ b/github/util_ruleset_validation.go @@ -23,25 +23,25 @@ import ( // 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", - "copilot_code_review", +var branchTagOnlyRules = []github.RepositoryRuleType{ + github.RulesetRuleTypeCreation, + github.RulesetRuleTypeUpdate, + github.RulesetRuleTypeDeletion, + github.RulesetRuleTypeRequiredLinearHistory, + github.RulesetRuleTypeRequiredSignatures, + github.RulesetRuleTypePullRequest, + github.RulesetRuleTypeRequiredStatusChecks, + github.RulesetRuleTypeNonFastForward, + github.RulesetRuleTypeCommitMessagePattern, + github.RulesetRuleTypeCommitAuthorEmailPattern, + github.RulesetRuleTypeCommitterEmailPattern, + github.RulesetRuleTypeBranchNamePattern, + github.RulesetRuleTypeTagNamePattern, + github.RulesetRuleTypeWorkflows, + github.RulesetRuleTypeCodeScanning, + github.RulesetRuleTypeRequiredDeployments, + github.RulesetRuleTypeMergeQueue, + github.RulesetRuleTypeCopilotCodeReview, } // pushOnlyRules contains rules that are only valid for push targets. @@ -56,11 +56,11 @@ var branchTagOnlyRules = []string{ // 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", +var pushOnlyRules = []github.RepositoryRuleType{ + github.RulesetRuleTypeFilePathRestriction, + github.RulesetRuleTypeMaxFilePathLength, + github.RulesetRuleTypeFileExtensionRestriction, + github.RulesetRuleTypeMaxFileSize, } func validateRulesForTarget(ctx context.Context, d *schema.ResourceDiff) error { @@ -86,7 +86,7 @@ func validateRulesForBranchTagTarget(ctx context.Context, d *schema.ResourceDiff return validateRules(ctx, d, branchTagOnlyRules) } -func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []string) error { +func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []github.RepositoryRuleType) error { target := github.RulesetTarget(d.Get("target").(string)) rules := d.Get("rules").([]any)[0].(map[string]any) for ruleName := range rules { @@ -108,7 +108,7 @@ func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []s continue } } - if slices.Contains(allowedRules, ruleName) { + if slices.Contains(allowedRules, github.RepositoryRuleType(ruleName)) { continue } else { tflog.Debug(ctx, fmt.Sprintf("Invalid rule for %s target", target), map[string]any{"rule": ruleName, "value": ruleValue}) From 5dd238cd41db6147ec8c98633699a9a4a9caa94c Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 00:44:36 +0200 Subject: [PATCH 44/56] Refactor `flattenRules` and `expandRules` to use context Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 6 +++--- ...source_github_organization_ruleset_test.go | 2 +- github/resource_github_repository_ruleset.go | 6 +++--- github/util_rules.go | 11 +++------- github/util_rules_test.go | 20 +++++++++---------- 5 files changed, 20 insertions(+), 25 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 3ac780f008..16b6f99b0e 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -654,7 +654,7 @@ func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.Reso _ = d.Set("ruleset_id", ruleset.ID) _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("etag", resp.Header.Get("ETag")) - _ = d.Set("rules", flattenRules(ruleset.Rules, true)) + _ = d.Set("rules", flattenRules(ctx, ruleset.Rules, true)) tflog.Info(ctx, fmt.Sprintf("Created organization ruleset: %s/%s (ID: %d)", owner, name, *ruleset.ID), map[string]any{ "owner": owner, @@ -721,8 +721,8 @@ 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", flattenConditionsWithContext(ctx, ruleset.GetConditions(), true)) - _ = d.Set("rules", flattenRules(ruleset.Rules, true)) + _ = d.Set("conditions", flattenConditions(ctx, ruleset.GetConditions(), true)) + _ = d.Set("rules", flattenRules(ctx, ruleset.Rules, true)) _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("etag", resp.Header.Get("ETag")) diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index ba3045cd3a..fb189c7c92 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -888,7 +888,7 @@ func TestOrganizationPushRulesetSupport(t *testing.T) { } // Test flatten functionality (organization rulesets use org=true) - flattenedResult := flattenRules(expandedRules, true) + flattenedResult := flattenRules(t.Context(), expandedRules, true) if len(flattenedResult) != 1 { t.Fatalf("Expected 1 flattened result, got %d", len(flattenedResult)) diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index f6f3389995..7d190cda4a 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -657,7 +657,7 @@ func resourceGithubRepositoryRulesetCreate(ctx context.Context, d *schema.Resour _ = d.Set("ruleset_id", ruleset.ID) _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("etag", resp.Header.Get("ETag")) - _ = d.Set("rules", flattenRules(ruleset.Rules, false)) + _ = d.Set("rules", flattenRules(ctx, ruleset.Rules, false)) return nil } @@ -704,8 +704,8 @@ func resourceGithubRepositoryRulesetRead(ctx context.Context, d *schema.Resource _ = d.Set("target", ruleset.GetTarget()) _ = d.Set("enforcement", ruleset.Enforcement) _ = d.Set("bypass_actors", flattenBypassActors(ruleset.BypassActors)) - _ = d.Set("conditions", flattenConditions(ruleset.GetConditions(), false)) - _ = d.Set("rules", flattenRules(ruleset.Rules, false)) + _ = d.Set("conditions", flattenConditions(ctx, ruleset.GetConditions(), false)) + _ = d.Set("rules", flattenRules(ctx, ruleset.Rules, false)) _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("etag", resp.Header.Get("ETag")) diff --git a/github/util_rules.go b/github/util_rules.go index 276b6eb895..ebf84e679f 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -2,7 +2,6 @@ package github import ( "context" - "log" "reflect" "sort" @@ -219,11 +218,7 @@ func expandConditions(input []any, org bool) *github.RepositoryRulesetConditions return rulesetConditions } -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 { +func flattenConditions(ctx context.Context, conditions *github.RepositoryRulesetConditions, org bool) []any { if conditions == nil || reflect.DeepEqual(conditions, &github.RepositoryRulesetConditions{}) { tflog.Debug(ctx, "Conditions are empty, returning empty list") return []any{} @@ -537,7 +532,7 @@ func expandRules(input []any, org bool) *github.RepositoryRulesetRules { return rulesetRules } -func flattenRules(rules *github.RepositoryRulesetRules, org bool) []any { +func flattenRules(ctx context.Context, rules *github.RepositoryRulesetRules, org bool) []any { if rules == nil { return []any{} } @@ -582,7 +577,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: %#v", pullRequestSlice) + tflog.Debug(ctx, "Flattened Pull Request rules slice", map[string]any{"pull_request": pullRequestSlice}) rulesMap["pull_request"] = pullRequestSlice } diff --git a/github/util_rules_test.go b/github/util_rules_test.go index d72cf76ad3..7ac73bb02c 100644 --- a/github/util_rules_test.go +++ b/github/util_rules_test.go @@ -57,7 +57,7 @@ func TestFlattenRulesBasicRules(t *testing.T) { NonFastForward: &github.EmptyRuleParameters{}, } - result := flattenRules(rules, false) + result := flattenRules(t.Context(), rules, false) if len(result) != 1 { t.Fatalf("Expected 1 element in result, got %d", len(result)) @@ -126,7 +126,7 @@ func TestFlattenRulesMaxFilePathLength(t *testing.T) { }, } - result := flattenRules(rules, false) + result := flattenRules(t.Context(), rules, false) if len(result) != 1 { t.Fatalf("Expected 1 element in result, got %d", len(result)) @@ -167,7 +167,7 @@ func TestRoundTripMaxFilePathLength(t *testing.T) { } // Flatten back to terraform format - flattenedResult := flattenRules(expandedRules, false) + flattenedResult := flattenRules(t.Context(), expandedRules, false) if len(flattenedResult) != 1 { t.Fatalf("Expected 1 flattened result, got %d", len(flattenedResult)) @@ -224,7 +224,7 @@ func TestFlattenRulesMaxFileSize(t *testing.T) { }, } - result := flattenRules(rules, false) + result := flattenRules(t.Context(), rules, false) if len(result) != 1 { t.Fatalf("Expected 1 element in result, got %d", len(result)) @@ -292,7 +292,7 @@ func TestFlattenRulesFileExtensionRestriction(t *testing.T) { }, } - result := flattenRules(rules, false) + result := flattenRules(t.Context(), rules, false) if len(result) != 1 { t.Fatalf("Expected 1 element in result, got %d", len(result)) @@ -372,7 +372,7 @@ func TestCompletePushRulesetSupport(t *testing.T) { } // Flatten back to terraform format - flattenedResult := flattenRules(expandedRules, false) + flattenedResult := flattenRules(t.Context(), expandedRules, false) if len(flattenedResult) != 1 { t.Fatalf("Expected 1 flattened result, got %d", len(flattenedResult)) @@ -452,7 +452,7 @@ func TestCopilotCodeReviewRoundTrip(t *testing.T) { } // Flatten back to terraform format - flattenedResult := flattenRules(expandedRules, false) + flattenedResult := flattenRules(t.Context(), expandedRules, false) if len(flattenedResult) != 1 { t.Fatalf("Expected 1 flattened result, got %d", len(flattenedResult)) @@ -485,7 +485,7 @@ func TestFlattenConditions_PushRuleset_WithRepositoryNameOnly(t *testing.T) { }, } - result := flattenConditions(conditions, true) // org=true for organization rulesets + result := flattenConditions(t.Context(), conditions, true) // org=true for organization rulesets if len(result) != 1 { t.Fatalf("Expected 1 conditions block, got %d", len(result)) @@ -531,7 +531,7 @@ func TestFlattenConditions_BranchRuleset_WithRefNameAndRepositoryName(t *testing }, } - result := flattenConditions(conditions, true) // org=true for organization rulesets + result := flattenConditions(t.Context(), conditions, true) // org=true for organization rulesets if len(result) != 1 { t.Fatalf("Expected 1 conditions block, got %d", len(result)) @@ -599,7 +599,7 @@ func TestFlattenConditions_PushRuleset_WithRepositoryIdOnly(t *testing.T) { }, } - result := flattenConditions(conditions, true) // org=true for organization rulesets + result := flattenConditions(t.Context(), conditions, true) // org=true for organization rulesets if len(result) != 1 { t.Fatalf("Expected 1 conditions block, got %d", len(result)) From 4e67141e9f579ad176c9a73920034953598f3a32 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 01:09:51 +0200 Subject: [PATCH 45/56] Fix failing validation Signed-off-by: Timo Sand --- github/util_ruleset_validation.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/github/util_ruleset_validation.go b/github/util_ruleset_validation.go index e8e6e74371..d8e4ce6fce 100644 --- a/github/util_ruleset_validation.go +++ b/github/util_ruleset_validation.go @@ -94,6 +94,9 @@ func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []g if !exists { continue } + if ruleName == "required_code_scanning" { + ruleName = string(github.RulesetRuleTypeCodeScanning) // This is one of the few rules which is not mapped to the same name in the API. + } switch ruleValue := ruleValue.(type) { case []any: if len(ruleValue) == 0 { From bde553dadb0f0dfcb674ba2cd1f20d677372c0dd Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 20:29:02 +0200 Subject: [PATCH 46/56] Refactor to use `ValidateDiagFunc` and `validation.ToDiagFunc` in repo ruleset Signed-off-by: Timo Sand --- github/resource_github_repository_ruleset.go | 60 ++++++++++---------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index 7d190cda4a..968ca946a6 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -15,6 +15,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) +var supportedRulesetTargetTypes = []string{string(github.RulesetTargetBranch), string(github.RulesetTargetPush), string(github.RulesetTargetTag)} + func resourceGithubRepositoryRuleset() *schema.Resource { return &schema.Resource{ CreateContext: resourceGithubRepositoryRulesetCreate, @@ -31,28 +33,28 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Schema: map[string]*schema.Schema{ "name": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringLenBetween(1, 100), - Description: "The name of the ruleset.", + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringLenBetween(1, 100)), + Description: "The name of the ruleset.", }, "target": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{string(github.RulesetTargetBranch), string(github.RulesetTargetPush), string(github.RulesetTargetTag)}, false), + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice(supportedRulesetTargetTypes, false)), Description: "Possible values are `branch`, `push` and `tag`.", }, "repository": { Type: schema.TypeString, Required: true, - ValidateDiagFunc: toDiagFunc(validation.StringMatch(regexp.MustCompile(`^[-a-zA-Z0-9_.]{1,100}$`), "must include only alphanumeric characters, underscores or hyphens and consist of 100 characters or less"), "name"), + ValidateDiagFunc: validation.ToDiagFunc(validation.StringMatch(regexp.MustCompile(`^[-a-zA-Z0-9_.]{1,100}$`), "must include only alphanumeric characters, underscores or hyphens and consist of 100 characters or less")), Description: "Name of the repository to apply ruleset to.", }, "enforcement": { - 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`.", + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: validation.ToDiagFunc(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`.", }, "bypass_actors": { Type: schema.TypeList, @@ -68,16 +70,16 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Description: "The ID of the actor that can bypass a ruleset. When `actor_type` is `OrganizationAdmin`, this should be set to `1`. Some resources such as DeployKey do not have an ID and this should be omitted.", }, "actor_type": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{"RepositoryRole", "Team", "Integration", "OrganizationAdmin", "DeployKey"}, false), - Description: "The type of actor that can bypass a ruleset. See https://docs.github.com/en/rest/repos/rules for more information.", + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"RepositoryRole", "Team", "Integration", "OrganizationAdmin", "DeployKey"}, false)), + Description: "The type of actor that can bypass a ruleset. See https://docs.github.com/en/rest/repos/rules for more information.", }, "bypass_mode": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{"always", "pull_request", "exempt"}, false), - Description: "When the specified actor can bypass the ruleset. pull_request means that an actor can only bypass rules on pull requests. Can be one of: `always`, `pull_request`, `exempt`.", + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"always", "pull_request", "exempt"}, false)), + Description: "When the specified actor can bypass the ruleset. pull_request means that an actor can only bypass rules on pull requests. Can be one of: `always`, `pull_request`, `exempt`.", }, }, }, @@ -199,7 +201,7 @@ func resourceGithubRepositoryRuleset() *schema.Resource { 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"), + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"merge", "squash", "rebase"}, false)), }, }, "dismiss_stale_reviews_on_push": { @@ -288,49 +290,49 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Type: schema.TypeInt, Optional: true, Default: 60, - ValidateDiagFunc: toDiagFunc(validation.IntBetween(0, 360), "check_response_timeout_minutes"), + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(0, 360)), Description: "Maximum time for a required status check to report a conclusion. After this much time has elapsed, checks that have not reported a conclusion will be assumed to have failed. Defaults to `60`.", }, "grouping_strategy": { Type: schema.TypeString, Optional: true, Default: "ALLGREEN", - ValidateDiagFunc: toDiagFunc(validation.StringInSlice([]string{"ALLGREEN", "HEADGREEN"}, false), "grouping_strategy"), + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"ALLGREEN", "HEADGREEN"}, false)), Description: "When set to ALLGREEN, the merge commit created by merge queue for each PR in the group must pass all required checks to merge. When set to HEADGREEN, only the commit at the head of the merge group, i.e. the commit containing changes from all of the PRs in the group, must pass its required checks to merge. Can be one of: ALLGREEN, HEADGREEN. Defaults to `ALLGREEN`.", }, "max_entries_to_build": { Type: schema.TypeInt, Optional: true, Default: 5, - ValidateDiagFunc: toDiagFunc(validation.IntBetween(0, 100), "max_entries_to_merge"), + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(0, 100)), Description: "Limit the number of queued pull requests requesting checks and workflow runs at the same time. Defaults to `5`.", }, "max_entries_to_merge": { Type: schema.TypeInt, Optional: true, Default: 5, - ValidateDiagFunc: toDiagFunc(validation.IntBetween(0, 100), "max_entries_to_merge"), + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(0, 100)), Description: "The maximum number of PRs that will be merged together in a group. Defaults to `5`.", }, "merge_method": { Type: schema.TypeString, Optional: true, Default: "MERGE", - ValidateDiagFunc: toDiagFunc(validation.StringInSlice([]string{"MERGE", "SQUASH", "REBASE"}, false), "merge_method"), + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"MERGE", "SQUASH", "REBASE"}, false)), Description: "Method to use when merging changes from queued pull requests. Can be one of: MERGE, SQUASH, REBASE. Defaults to `MERGE`.", }, "min_entries_to_merge": { Type: schema.TypeInt, Optional: true, Default: 1, - ValidateDiagFunc: toDiagFunc(validation.IntBetween(0, 100), "min_entries_to_merge"), + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(0, 100)), Description: "The minimum number of PRs that will be merged together in a group. Defaults to `1`.", }, "min_entries_to_merge_wait_minutes": { Type: schema.TypeInt, Optional: true, Default: 5, - ValidateDiagFunc: toDiagFunc(validation.IntBetween(0, 360), "min_entries_to_merge_wait_minutes"), + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(0, 360)), Description: "The time merge queue should wait after the first PR is added to the queue for the minimum group size to be met. After this time has elapsed, the minimum group size will be ignored and a smaller group will be merged. Defaults to `5`.", }, }, @@ -558,7 +560,7 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Type: schema.TypeInt, Required: true, Description: "The maximum allowed size of a file in megabytes (MB). Valid range is 1-100 MB.", - ValidateDiagFunc: toDiagFunc(validation.IntBetween(1, 100), "max_file_size"), + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 100)), }, }, }, From 05be1aed20505106e5b01cf1c7058c8906606253 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 20:29:49 +0200 Subject: [PATCH 47/56] Use `strings.Join` to make `Description` of `target` dynamic Signed-off-by: Timo Sand --- github/resource_github_repository_ruleset.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index 968ca946a6..e47d7f0daa 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -7,6 +7,7 @@ import ( "net/http" "regexp" "strconv" + "strings" "github.com/google/go-github/v82/github" "github.com/hashicorp/terraform-plugin-log/tflog" @@ -42,7 +43,7 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Type: schema.TypeString, Required: true, ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice(supportedRulesetTargetTypes, false)), - Description: "Possible values are `branch`, `push` and `tag`.", + Description: "Possible values are " + strings.Join(supportedRulesetTargetTypes[:len(supportedRulesetTargetTypes)-1], ", ") + " and " + supportedRulesetTargetTypes[len(supportedRulesetTargetTypes)-1], }, "repository": { Type: schema.TypeString, From 88836c05bd34762f2958854cf915502c28cb59e8 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 20:34:17 +0200 Subject: [PATCH 48/56] Add validation to `operator` attributes Signed-off-by: Timo Sand --- github/resource_github_repository_ruleset.go | 37 ++++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index e47d7f0daa..be780c9549 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -18,6 +18,8 @@ import ( var supportedRulesetTargetTypes = []string{string(github.RulesetTargetBranch), string(github.RulesetTargetPush), string(github.RulesetTargetTag)} +var operatorValidation = validation.ToDiagFunc(validation.StringInSlice([]string{"starts_with", "ends_with", "contains", "regex"}, false)) + func resourceGithubRepositoryRuleset() *schema.Resource { return &schema.Resource{ CreateContext: resourceGithubRepositoryRulesetCreate, @@ -362,9 +364,10 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Description: "If true, the rule will fail if the pattern matches.", }, "operator": { - Type: schema.TypeString, - Required: true, - Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", + Type: schema.TypeString, + ValidateDiagFunc: operatorValidation, + Required: true, + Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", }, "pattern": { Type: schema.TypeString, @@ -392,9 +395,10 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Description: "If true, the rule will fail if the pattern matches.", }, "operator": { - Type: schema.TypeString, - Required: true, - Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", + Type: schema.TypeString, + ValidateDiagFunc: operatorValidation, + Required: true, + Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", }, "pattern": { Type: schema.TypeString, @@ -422,9 +426,10 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Description: "If true, the rule will fail if the pattern matches.", }, "operator": { - Type: schema.TypeString, - Required: true, - Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", + Type: schema.TypeString, + ValidateDiagFunc: operatorValidation, + Required: true, + Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", }, "pattern": { Type: schema.TypeString, @@ -453,9 +458,10 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Description: "If true, the rule will fail if the pattern matches.", }, "operator": { - Type: schema.TypeString, - Required: true, - Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", + Type: schema.TypeString, + ValidateDiagFunc: operatorValidation, + Required: true, + Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", }, "pattern": { Type: schema.TypeString, @@ -484,9 +490,10 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Description: "If true, the rule will fail if the pattern matches.", }, "operator": { - Type: schema.TypeString, - Required: true, - Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", + Type: schema.TypeString, + ValidateDiagFunc: operatorValidation, + Required: true, + Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", }, "pattern": { Type: schema.TypeString, From 40056651c413c7b0cbba0a20f2097aba85e3e56b Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 20:37:43 +0200 Subject: [PATCH 49/56] Add missing validations to repo ruleset attributes Signed-off-by: Timo Sand --- github/resource_github_repository_ruleset.go | 30 +++++++++++--------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index be780c9549..8322dbb980 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -226,10 +226,11 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Description: "Whether the most recent reviewable push must be approved by someone other than the person who pushed it. Defaults to `false`.", }, "required_approving_review_count": { - Type: schema.TypeInt, - Optional: true, - Default: 0, - Description: "The number of approving reviews that are required before a pull request can be merged. Defaults to `0`.", + Type: schema.TypeInt, + Optional: true, + Default: 0, + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(0, 10)), + Description: "The number of approving reviews that are required before a pull request can be merged. Defaults to `0`.", }, "required_review_thread_resolution": { Type: schema.TypeBool, @@ -518,14 +519,16 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "alerts_threshold": { - Type: schema.TypeString, - Required: true, - Description: "The severity level at which code scanning results that raise alerts block a reference update. Can be one of: `none`, `errors`, `errors_and_warnings`, `all`.", + Description: "The severity level at which code scanning results that raise alerts block a reference update. Can be one of: `none`, `errors`, `errors_and_warnings`, `all`.", + Required: true, + Type: schema.TypeString, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"none", "errors", "errors_and_warnings", "all"}, false)), }, "security_alerts_threshold": { - Type: schema.TypeString, - Required: true, - Description: "The severity level at which code scanning results that raise security alerts block a reference update. Can be one of: `none`, `critical`, `high_or_higher`, `medium_or_higher`, `all`.", + Description: "The severity level at which code scanning results that raise security alerts block a reference update. Can be one of: `none`, `critical`, `high_or_higher`, `medium_or_higher`, `all`.", + Required: true, + Type: schema.TypeString, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"none", "critical", "high_or_higher", "medium_or_higher", "all"}, false)), }, "tool": { Type: schema.TypeString, @@ -581,9 +584,10 @@ func resourceGithubRepositoryRuleset() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "max_file_path_length": { - Type: schema.TypeInt, - Required: true, - Description: "The maximum allowed length of a file path.", + Type: schema.TypeInt, + Required: true, + Description: "The maximum allowed length of a file path.", + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 32767)), }, }, }, From 229752fdd98eb9943e5011b4c17da47c1efc0b76 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 22:21:23 +0200 Subject: [PATCH 50/56] Refactor to use `ValidateDiagFunc` and `validation.ToDiagFunc` in org ruleset --- .../resource_github_organization_ruleset.go | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 16b6f99b0e..a070d468a2 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -31,23 +31,23 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Schema: map[string]*schema.Schema{ "name": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringLenBetween(1, 100), - Description: "The name of the ruleset.", + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringLenBetween(1, 100)), + Description: "The name of the ruleset.", }, "target": { Type: schema.TypeString, Required: true, // The API accepts an `repository` target, but we don't support it yet. - ValidateFunc: validation.StringInSlice([]string{string(github.RulesetTargetBranch), string(github.RulesetTargetTag), string(github.RulesetTargetPush)}, false), - Description: "The target of the ruleset. Possible values are `branch`, `tag` and `push`.", + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{string(github.RulesetTargetBranch), string(github.RulesetTargetTag), string(github.RulesetTargetPush)}, false)), + Description: "The target of the ruleset. Possible values are `branch`, `tag` and `push`.", }, "enforcement": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{"disabled", "active", "evaluate"}, false), - 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.", + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"disabled", "active", "evaluate"}, false)), + 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 @@ -63,16 +63,16 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "The ID of the actor that can bypass a ruleset. When `actor_type` is `OrganizationAdmin`, this should be set to `1`. Some resources such as DeployKey do not have an ID and this should be omitted.", }, "actor_type": { - 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. Can be one of: `Integration`, `OrganizationAdmin`, `RepositoryRole`, `Team`, or `DeployKey`.", + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"Integration", "OrganizationAdmin", "RepositoryRole", "Team", "DeployKey"}, false)), + 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, - Required: true, - ValidateFunc: validation.StringInSlice([]string{"always", "pull_request", "exempt"}, false), - Description: "When the specified actor can bypass the ruleset. pull_request means that an actor can only bypass rules on pull requests. Can be one of: `always`, `pull_request`, `exempt`.", + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"always", "pull_request", "exempt"}, false)), + Description: "When the specified actor can bypass the ruleset. pull_request means that an actor can only bypass rules on pull requests. Can be one of: `always`, `pull_request`, `exempt`.", }, }, }, @@ -212,7 +212,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { 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"), + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"merge", "squash", "rebase"}, false)), }, }, "dismiss_stale_reviews_on_push": { @@ -497,7 +497,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { "path": { Type: schema.TypeString, Required: true, - ValidateDiagFunc: toDiagFunc(validation.StringMatch(regexp.MustCompile(`^\.github\/workflows\/.*$`), "Path must be in the .github/workflows directory"), "path"), + ValidateDiagFunc: validation.ToDiagFunc(validation.StringMatch(regexp.MustCompile(`^\.github\/workflows\/.*$`), "Path must be in the .github/workflows directory")), Description: "The path to the workflow YAML definition file.", }, "ref": { @@ -577,7 +577,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Type: schema.TypeInt, Required: true, Description: "The maximum allowed size of a file in megabytes (MB). Valid range is 1-100 MB.", - ValidateDiagFunc: toDiagFunc(validation.IntBetween(1, 100), "max_file_size"), + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 100)), }, }, }, From 714752609b5a4d4a014b65abbf8f92c7306251cb Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 22:38:02 +0200 Subject: [PATCH 51/56] Use `strings.Join` to make `Description` of `target` dynamic --- github/resource_github_organization_ruleset.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index a070d468a2..9d79292974 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -7,6 +7,7 @@ import ( "net/http" "regexp" "strconv" + "strings" "github.com/google/go-github/v82/github" "github.com/hashicorp/terraform-plugin-log/tflog" @@ -15,6 +16,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) +var supportedOrgRulesetTargetTypes = []string{string(github.RulesetTargetBranch), string(github.RulesetTargetTag), string(github.RulesetTargetPush)} + func resourceGithubOrganizationRuleset() *schema.Resource { return &schema.Resource{ CreateContext: resourceGithubOrganizationRulesetCreate, @@ -40,8 +43,8 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Type: schema.TypeString, Required: true, // The API accepts an `repository` target, but we don't support it yet. - ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{string(github.RulesetTargetBranch), string(github.RulesetTargetTag), string(github.RulesetTargetPush)}, false)), - Description: "The target of the ruleset. Possible values are `branch`, `tag` and `push`.", + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice(supportedOrgRulesetTargetTypes, false)), + Description: "The target of the ruleset. Possible values are " + strings.Join(supportedOrgRulesetTargetTypes[:len(supportedOrgRulesetTargetTypes)-1], ", ") + " and " + supportedOrgRulesetTargetTypes[len(supportedOrgRulesetTargetTypes)-1] + ".", }, "enforcement": { Type: schema.TypeString, From dc8953e2706c63ad3912c7e3aa3ce92813a26e04 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 23:16:55 +0200 Subject: [PATCH 52/56] Add validation to `operator` attributes --- .../resource_github_organization_ruleset.go | 35 +++++++++++-------- github/resource_github_repository_ruleset.go | 2 -- github/util_ruleset_validation.go | 3 ++ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 9d79292974..f86e56ee7c 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -339,9 +339,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "If true, the rule will fail if the pattern matches.", }, "operator": { - Type: schema.TypeString, - Required: true, - Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", + Type: schema.TypeString, + ValidateDiagFunc: operatorValidation, + Required: true, + Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", }, "pattern": { Type: schema.TypeString, @@ -369,9 +370,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "If true, the rule will fail if the pattern matches.", }, "operator": { - Type: schema.TypeString, - Required: true, - Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", + Type: schema.TypeString, + ValidateDiagFunc: operatorValidation, + Required: true, + Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", }, "pattern": { Type: schema.TypeString, @@ -399,9 +401,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "If true, the rule will fail if the pattern matches.", }, "operator": { - Type: schema.TypeString, - Required: true, - Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", + Type: schema.TypeString, + ValidateDiagFunc: operatorValidation, + Required: true, + Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", }, "pattern": { Type: schema.TypeString, @@ -430,9 +433,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "If true, the rule will fail if the pattern matches.", }, "operator": { - Type: schema.TypeString, - Required: true, - Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", + Type: schema.TypeString, + ValidateDiagFunc: operatorValidation, + Required: true, + Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", }, "pattern": { Type: schema.TypeString, @@ -461,9 +465,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "If true, the rule will fail if the pattern matches.", }, "operator": { - Type: schema.TypeString, - Required: true, - Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", + Type: schema.TypeString, + ValidateDiagFunc: operatorValidation, + Required: true, + Description: "The operator to use for matching. Can be one of: `starts_with`, `ends_with`, `contains`, `regex`.", }, "pattern": { Type: schema.TypeString, diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index 8322dbb980..e6c45252af 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -18,8 +18,6 @@ import ( var supportedRulesetTargetTypes = []string{string(github.RulesetTargetBranch), string(github.RulesetTargetPush), string(github.RulesetTargetTag)} -var operatorValidation = validation.ToDiagFunc(validation.StringInSlice([]string{"starts_with", "ends_with", "contains", "regex"}, false)) - func resourceGithubRepositoryRuleset() *schema.Resource { return &schema.Resource{ CreateContext: resourceGithubRepositoryRulesetCreate, diff --git a/github/util_ruleset_validation.go b/github/util_ruleset_validation.go index d8e4ce6fce..3ef8305c52 100644 --- a/github/util_ruleset_validation.go +++ b/github/util_ruleset_validation.go @@ -8,8 +8,11 @@ import ( "github.com/google/go-github/v82/github" "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) +var operatorValidation = validation.ToDiagFunc(validation.StringInSlice([]string{"starts_with", "ends_with", "contains", "regex"}, false)) + // 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 From 02b402ad9b1441381215db528658a14eabfae95a Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 23:40:55 +0200 Subject: [PATCH 53/56] Add missing validations to org ruleset attributes --- .../resource_github_organization_ruleset.go | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index f86e56ee7c..b0cc71f448 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -237,10 +237,11 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Description: "Whether the most recent reviewable push must be approved by someone other than the person who pushed it. Defaults to `false`.", }, "required_approving_review_count": { - Type: schema.TypeInt, - Optional: true, - Default: 0, - Description: "The number of approving reviews that are required before a pull request can be merged. Defaults to `0`.", + Type: schema.TypeInt, + Optional: true, + Default: 0, + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(0, 10)), + Description: "The number of approving reviews that are required before a pull request can be merged. Defaults to `0`.", }, "required_review_thread_resolution": { Type: schema.TypeBool, @@ -535,14 +536,16 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "alerts_threshold": { - Type: schema.TypeString, - Required: true, - Description: "The severity level at which code scanning results that raise alerts block a reference update. Can be one of: `none`, `errors`, `errors_and_warnings`, `all`.", + Description: "The severity level at which code scanning results that raise alerts block a reference update. Can be one of: `none`, `errors`, `errors_and_warnings`, `all`.", + Required: true, + Type: schema.TypeString, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"none", "errors", "errors_and_warnings", "all"}, false)), }, "security_alerts_threshold": { - Type: schema.TypeString, - Required: true, - Description: "The severity level at which code scanning results that raise security alerts block a reference update. Can be one of: `none`, `critical`, `high_or_higher`, `medium_or_higher`, `all`.", + Description: "The severity level at which code scanning results that raise security alerts block a reference update. Can be one of: `none`, `critical`, `high_or_higher`, `medium_or_higher`, `all`.", + Required: true, + Type: schema.TypeString, + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{"none", "critical", "high_or_higher", "medium_or_higher", "all"}, false)), }, "tool": { Type: schema.TypeString, @@ -598,9 +601,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "max_file_path_length": { - Type: schema.TypeInt, - Required: true, - Description: "The maximum allowed length of a file path.", + Type: schema.TypeInt, + Required: true, + Description: "The maximum allowed length of a file path.", + ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 32767)), }, }, }, From cc7d1f23812efc7f9d164ee17f5fdf03029181f6 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 30 Jan 2026 23:48:14 +0200 Subject: [PATCH 54/56] Ensure attribute names map correctly to rule names Signed-off-by: Timo Sand --- github/util_ruleset_validation.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/github/util_ruleset_validation.go b/github/util_ruleset_validation.go index 3ef8305c52..2ee0254fc1 100644 --- a/github/util_ruleset_validation.go +++ b/github/util_ruleset_validation.go @@ -97,8 +97,12 @@ func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []g if !exists { continue } - if ruleName == "required_code_scanning" { - ruleName = string(github.RulesetRuleTypeCodeScanning) // This is one of the few rules which is not mapped to the same name in the API. + // These are the few rules which are not mapped to the same name in the API. + switch ruleName { + case "required_code_scanning": + ruleName = string(github.RulesetRuleTypeCodeScanning) + case "required_workflows": + ruleName = string(github.RulesetRuleTypeWorkflows) } switch ruleValue := ruleValue.(type) { case []any: From c2a8bb3a16c21ee3a36ba69bf31e5a5e9f632597 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 31 Jan 2026 00:02:12 +0200 Subject: [PATCH 55/56] Don't ignore errors Signed-off-by: Timo Sand --- .../resource_github_organization_ruleset.go | 64 ++++++++++++----- github/resource_github_repository_ruleset.go | 72 ++++++++++++++----- 2 files changed, 101 insertions(+), 35 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index b0cc71f448..f26512a466 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -663,10 +663,18 @@ func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.Reso } d.SetId(strconv.FormatInt(*ruleset.ID, 10)) - _ = d.Set("ruleset_id", ruleset.ID) - _ = d.Set("node_id", ruleset.GetNodeID()) - _ = d.Set("etag", resp.Header.Get("ETag")) - _ = d.Set("rules", flattenRules(ctx, ruleset.Rules, true)) + if err := d.Set("ruleset_id", ruleset.ID); err != nil { + return diag.FromErr(err) + } + if err := d.Set("node_id", ruleset.GetNodeID()); err != nil { + return diag.FromErr(err) + } + if err := d.Set("etag", resp.Header.Get("ETag")); err != nil { + return diag.FromErr(err) + } + if err := d.Set("rules", flattenRules(ctx, ruleset.Rules, true)); err != nil { + return diag.FromErr(err) + } tflog.Info(ctx, fmt.Sprintf("Created organization ruleset: %s/%s (ID: %d)", owner, name, *ruleset.ID), map[string]any{ "owner": owner, @@ -728,15 +736,33 @@ func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.Resour return diag.FromErr(err) } - _ = d.Set("ruleset_id", ruleset.ID) - _ = d.Set("name", ruleset.Name) - _ = d.Set("target", ruleset.GetTarget()) - _ = d.Set("enforcement", ruleset.Enforcement) - _ = d.Set("bypass_actors", flattenBypassActors(ruleset.BypassActors)) - _ = d.Set("conditions", flattenConditions(ctx, ruleset.GetConditions(), true)) - _ = d.Set("rules", flattenRules(ctx, ruleset.Rules, true)) - _ = d.Set("node_id", ruleset.GetNodeID()) - _ = d.Set("etag", resp.Header.Get("ETag")) + if err := d.Set("ruleset_id", ruleset.ID); err != nil { + return diag.FromErr(err) + } + if err := d.Set("name", ruleset.Name); err != nil { + return diag.FromErr(err) + } + if err := d.Set("target", ruleset.GetTarget()); err != nil { + return diag.FromErr(err) + } + if err := d.Set("enforcement", ruleset.Enforcement); err != nil { + return diag.FromErr(err) + } + if err := d.Set("bypass_actors", flattenBypassActors(ruleset.BypassActors)); err != nil { + return diag.FromErr(err) + } + if err := d.Set("conditions", flattenConditions(ctx, ruleset.GetConditions(), true)); err != nil { + return diag.FromErr(err) + } + if err := d.Set("rules", flattenRules(ctx, ruleset.Rules, true)); err != nil { + return diag.FromErr(err) + } + if err := d.Set("node_id", ruleset.GetNodeID()); err != nil { + return diag.FromErr(err) + } + if err := d.Set("etag", resp.Header.Get("ETag")); err != nil { + return diag.FromErr(err) + } tflog.Trace(ctx, fmt.Sprintf("Successfully read organization ruleset: %s/%d", owner, rulesetID), map[string]any{ "owner": owner, @@ -781,9 +807,15 @@ func resourceGithubOrganizationRulesetUpdate(ctx context.Context, d *schema.Reso } d.SetId(strconv.FormatInt(*ruleset.ID, 10)) - _ = d.Set("ruleset_id", ruleset.ID) - _ = d.Set("node_id", ruleset.GetNodeID()) - _ = d.Set("etag", resp.Header.Get("ETag")) + if err := d.Set("ruleset_id", ruleset.ID); err != nil { + return diag.FromErr(err) + } + if err := d.Set("node_id", ruleset.GetNodeID()); err != nil { + return diag.FromErr(err) + } + if err := d.Set("etag", resp.Header.Get("ETag")); err != nil { + return diag.FromErr(err) + } tflog.Info(ctx, fmt.Sprintf("Updated organization ruleset: %s/%d", owner, rulesetID), map[string]any{ "owner": owner, diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index e6c45252af..bd92d6be3c 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -665,11 +665,19 @@ func resourceGithubRepositoryRulesetCreate(ctx context.Context, d *schema.Resour return diag.FromErr(err) } - d.SetId(strconv.FormatInt(*ruleset.ID, 10)) - _ = d.Set("ruleset_id", ruleset.ID) - _ = d.Set("node_id", ruleset.GetNodeID()) - _ = d.Set("etag", resp.Header.Get("ETag")) - _ = d.Set("rules", flattenRules(ctx, ruleset.Rules, false)) + d.SetId(strconv.FormatInt(ruleset.GetID(), 10)) + if err := d.Set("ruleset_id", ruleset.GetID()); err != nil { + return diag.FromErr(err) + } + if err := d.Set("node_id", ruleset.GetNodeID()); err != nil { + return diag.FromErr(err) + } + if err := d.Set("etag", resp.Header.Get("ETag")); err != nil { + return diag.FromErr(err) + } + if err := d.Set("rules", flattenRules(ctx, ruleset.Rules, false)); err != nil { + return diag.FromErr(err) + } return nil } @@ -711,15 +719,33 @@ func resourceGithubRepositoryRulesetRead(ctx context.Context, d *schema.Resource return nil } - _ = d.Set("ruleset_id", ruleset.ID) - _ = d.Set("name", ruleset.Name) - _ = d.Set("target", ruleset.GetTarget()) - _ = d.Set("enforcement", ruleset.Enforcement) - _ = d.Set("bypass_actors", flattenBypassActors(ruleset.BypassActors)) - _ = d.Set("conditions", flattenConditions(ctx, ruleset.GetConditions(), false)) - _ = d.Set("rules", flattenRules(ctx, ruleset.Rules, false)) - _ = d.Set("node_id", ruleset.GetNodeID()) - _ = d.Set("etag", resp.Header.Get("ETag")) + if err := d.Set("ruleset_id", ruleset.GetID()); err != nil { + return diag.FromErr(err) + } + if err := d.Set("name", ruleset.Name); err != nil { + return diag.FromErr(err) + } + if err := d.Set("target", ruleset.GetTarget()); err != nil { + return diag.FromErr(err) + } + if err := d.Set("enforcement", ruleset.Enforcement); err != nil { + return diag.FromErr(err) + } + if err := d.Set("bypass_actors", flattenBypassActors(ruleset.BypassActors)); err != nil { + return diag.FromErr(err) + } + if err := d.Set("conditions", flattenConditions(ctx, ruleset.GetConditions(), false)); err != nil { + return diag.FromErr(err) + } + if err := d.Set("rules", flattenRules(ctx, ruleset.GetRules(), false)); err != nil { + return diag.FromErr(err) + } + if err := d.Set("node_id", ruleset.GetNodeID()); err != nil { + return diag.FromErr(err) + } + if err := d.Set("etag", resp.Header.Get("ETag")); err != nil { + return diag.FromErr(err) + } return nil } @@ -752,9 +778,15 @@ func resourceGithubRepositoryRulesetUpdate(ctx context.Context, d *schema.Resour return diag.FromErr(err) } d.SetId(strconv.FormatInt(*ruleset.ID, 10)) - _ = d.Set("ruleset_id", ruleset.ID) - _ = d.Set("node_id", ruleset.GetNodeID()) - _ = d.Set("etag", resp.Header.Get("ETag")) + if err := d.Set("ruleset_id", ruleset.ID); err != nil { + return diag.FromErr(err) + } + if err := d.Set("node_id", ruleset.GetNodeID()); err != nil { + return diag.FromErr(err) + } + if err := d.Set("etag", resp.Header.Get("ETag")); err != nil { + return diag.FromErr(err) + } return nil } @@ -796,9 +828,11 @@ func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.Resour if repository == nil || err != nil { return []*schema.ResourceData{d}, err } - _ = d.Set("repository", *repository.Name) + if err := d.Set("repository", repository.GetName()); err != nil { + return []*schema.ResourceData{d}, err + } - ruleset, _, err := client.Repositories.GetRuleset(ctx, owner, *repository.Name, rulesetID, false) + ruleset, _, err := client.Repositories.GetRuleset(ctx, owner, repository.GetName(), rulesetID, false) if ruleset == nil || err != nil { return []*schema.ResourceData{d}, err } From 00afcdb4585faf88025b76dbeb40edae2a98ffb6 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 3 Feb 2026 16:21:48 +0200 Subject: [PATCH 56/56] Address review comments Signed-off-by: Timo Sand --- github/resource_github_organization_ruleset.go | 8 ++++---- github/resource_github_repository_ruleset.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index f26512a466..b225b90a69 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -662,7 +662,7 @@ func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.Reso return diag.FromErr(err) } - d.SetId(strconv.FormatInt(*ruleset.ID, 10)) + d.SetId(strconv.FormatInt(ruleset.GetID(), 10)) if err := d.Set("ruleset_id", ruleset.ID); err != nil { return diag.FromErr(err) } @@ -676,10 +676,10 @@ func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.Reso return diag.FromErr(err) } - tflog.Info(ctx, fmt.Sprintf("Created organization ruleset: %s/%s (ID: %d)", owner, name, *ruleset.ID), map[string]any{ + tflog.Info(ctx, fmt.Sprintf("Created organization ruleset: %s/%s (ID: %d)", owner, name, ruleset.GetID()), map[string]any{ "owner": owner, "name": name, - "ruleset_id": *ruleset.ID, + "ruleset_id": ruleset.GetID(), }) return nil @@ -806,7 +806,7 @@ func resourceGithubOrganizationRulesetUpdate(ctx context.Context, d *schema.Reso return diag.FromErr(err) } - d.SetId(strconv.FormatInt(*ruleset.ID, 10)) + d.SetId(strconv.FormatInt(ruleset.GetID(), 10)) if err := d.Set("ruleset_id", ruleset.ID); err != nil { return diag.FromErr(err) } diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index bd92d6be3c..eacef5f781 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -666,7 +666,7 @@ func resourceGithubRepositoryRulesetCreate(ctx context.Context, d *schema.Resour } d.SetId(strconv.FormatInt(ruleset.GetID(), 10)) - if err := d.Set("ruleset_id", ruleset.GetID()); err != nil { + if err := d.Set("ruleset_id", ruleset.ID); err != nil { return diag.FromErr(err) } if err := d.Set("node_id", ruleset.GetNodeID()); err != nil { @@ -719,7 +719,7 @@ func resourceGithubRepositoryRulesetRead(ctx context.Context, d *schema.Resource return nil } - if err := d.Set("ruleset_id", ruleset.GetID()); err != nil { + if err := d.Set("ruleset_id", ruleset.ID); err != nil { return diag.FromErr(err) } if err := d.Set("name", ruleset.Name); err != nil { @@ -777,7 +777,7 @@ func resourceGithubRepositoryRulesetUpdate(ctx context.Context, d *schema.Resour if err != nil { return diag.FromErr(err) } - d.SetId(strconv.FormatInt(*ruleset.ID, 10)) + d.SetId(strconv.FormatInt(ruleset.GetID(), 10)) if err := d.Set("ruleset_id", ruleset.ID); err != nil { return diag.FromErr(err) }