|FEAT] Add ruleset rule for pull request required reviewers

github/repository_rules_validation_utils.go

@@ -0,0 +1,157 @@
+package github
+
+import (
+	"context"
+	"fmt"
+	"slices"
+
+	"github.com/hashicorp/terraform-plugin-log/tflog"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+)
+
+// branchTagOnlyRules contains rules that are only valid for branch and tag targets.
+//
+// These rules apply to ref-based operations (branches and tags) and are not supported
+// for push rulesets which operate on file content.
+//
+// To verify/maintain this list:
+//  1. Check the GitHub API documentation for organization rulesets:
+//     https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-ruleset
+//  2. The API docs don't clearly separate push vs branch/tag rules. To verify,
+//     attempt to create a push ruleset via API or UI with each rule type.
+//     Push rulesets will reject branch/tag rules with "Invalid rule '<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",
+	"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
+}

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)
+			}
+		}
+	}
+}