Add validation for push rules

As they differ from `branch` and `tag` rules

Signed-off-by: Timo Sand <[email protected]>

Author: deiga

github/resource_github_organization_ruleset.go

@@ -12,6 +12,7 @@ import (
 	"github.com/google/go-github/v77/github"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 )
@@ -28,7 +29,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource {
 
 		SchemaVersion: 1,
 
-		CustomizeDiff: validateConditionsFieldBasedOnTarget,
+		CustomizeDiff: customdiff.All(
+			validateConditionsFieldBasedOnTarget,
+			validateRulesFieldBasedOnTarget,
+		),
 
 		Schema: map[string]*schema.Schema{
 			"name": {
@@ -781,3 +785,124 @@ func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.Resourc
 	}
 	return nil
 }
+
+// branchTagOnlyRules contains rules that are only valid for branch and tag targets.
+//
+// These rules apply to ref-based operations (branches and tags) and are not supported
+// for push rulesets which operate on file content.
+//
+// To verify/maintain this list:
+//  1. Check the GitHub API documentation for organization rulesets:
+//     https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-ruleset
+//  2. The API docs don't clearly separate push vs branch/tag rules. To verify,
+//     attempt to create a push ruleset via API or UI with each rule type.
+//     Push rulesets will reject branch/tag rules with "Invalid rule '<name>'" 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
+}

github/resource_github_organization_ruleset_test.go

@@ -739,6 +739,100 @@ func TestGithubOrganizationRulesets(t *testing.T) {
 		})
 	})
 
+	t.Run("Validates push target rejects branch/tag rules", func(t *testing.T) {
+		resourceName := "test-push-reject-branch-rules"
+		config := fmt.Sprintf(`
+			resource "github_organization_ruleset" "%s" {
+				name        = "test-push-branch-rule-%s"
+				target      = "push"
+				enforcement = "active"
+
+				conditions {
+					repository_name {
+						include = ["~ALL"]
+						exclude = []
+					}
+				}
+
+				rules {
+					# 'creation' is a branch/tag rule, not valid for push target
+					creation = true
+				}
+			}
+		`, resourceName, randomID)
+
+		testCase := func(t *testing.T, mode string) {
+			resource.Test(t, resource.TestCase{
+				PreCheck:  func() { skipUnlessMode(t, mode) },
+				Providers: testAccProviders,
+				Steps: []resource.TestStep{
+					{
+						Config:      config,
+						ExpectError: regexp.MustCompile("rule .* is not valid for push target"),
+					},
+				},
+			})
+		}
+
+		t.Run("with an enterprise account", func(t *testing.T) {
+			testCase(t, enterprise)
+		})
+
+		t.Run("with an organization account", func(t *testing.T) {
+			t.Skip("organization account not supported for this operation, since it needs a paid Team plan.")
+		})
+	})
+
+	t.Run("Validates branch target rejects push-only rules", func(t *testing.T) {
+		resourceName := "test-branch-reject-push-rules"
+		config := fmt.Sprintf(`
+			resource "github_organization_ruleset" "%s" {
+				name        = "test-branch-push-rule-%s"
+				target      = "branch"
+				enforcement = "active"
+
+				conditions {
+					ref_name {
+						include = ["~ALL"]
+						exclude = []
+					}
+					repository_name {
+						include = ["~ALL"]
+						exclude = []
+					}
+				}
+
+				rules {
+					# 'max_file_size' is a push-only rule, not valid for branch target
+					max_file_size {
+						max_file_size = 100
+					}
+				}
+			}
+		`, resourceName, randomID)
+
+		testCase := func(t *testing.T, mode string) {
+			resource.Test(t, resource.TestCase{
+				PreCheck:  func() { skipUnlessMode(t, mode) },
+				Providers: testAccProviders,
+				Steps: []resource.TestStep{
+					{
+						Config:      config,
+						ExpectError: regexp.MustCompile("rule .* is only valid for push target"),
+					},
+				},
+			})
+		}
+
+		t.Run("with an enterprise account", func(t *testing.T) {
+			testCase(t, enterprise)
+		})
+
+		t.Run("with an organization account", func(t *testing.T) {
+			t.Skip("organization account not supported for this operation, since it needs a paid Team plan.")
+		})
+	})
+
 	t.Run("Creates push ruleset with repository_name only", func(t *testing.T) {
 		resourceName := "test-push-repo-name-only"
 		config := fmt.Sprintf(`