Skip to content

Commit a1a9a68

Browse files
deigadeiga
committed
Extract validation functions to separate utils file with unit tests
Signed-off-by: Timo Sand <[email protected]>
1 parent 1a50eef commit a1a9a68

5 files changed

Lines changed: 417 additions & 208 deletions

‎github/repository_rules_validation_utils.go‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
package github
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"slices"
7+
8+
"github.com/hashicorp/terraform-plugin-log/tflog"
9+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
10+
)
11+
12+
// branchTagOnlyRules contains rules that are only valid for branch and tag targets.
13+
//
14+
// These rules apply to ref-based operations (branches and tags) and are not supported
15+
// for push rulesets which operate on file content.
16+
//
17+
// To verify/maintain this list:
18+
// 1. Check the GitHub API documentation for organization rulesets:
19+
// https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-ruleset
20+
// 2. The API docs don't clearly separate push vs branch/tag rules. To verify,
21+
// attempt to create a push ruleset via API or UI with each rule type.
22+
// Push rulesets will reject branch/tag rules with "Invalid rule '<name>'" error.
23+
// 3. Generally, push rules deal with file content (paths, sizes, extensions),
24+
// while branch/tag rules deal with ref lifecycle and merge requirements.
25+
var branchTagOnlyRules = []string{
26+
"creation",
27+
"update",
28+
"deletion",
29+
"required_linear_history",
30+
"required_signatures",
31+
"pull_request",
32+
"required_status_checks",
33+
"non_fast_forward",
34+
"commit_message_pattern",
35+
"commit_author_email_pattern",
36+
"committer_email_pattern",
37+
"branch_name_pattern",
38+
"tag_name_pattern",
39+
"required_workflows",
40+
"required_code_scanning",
41+
"required_deployments",
42+
"merge_queue",
43+
}
44+
45+
// pushOnlyRules contains rules that are only valid for push targets.
46+
//
47+
// These rules apply to push operations and control what content can be pushed
48+
// to repositories. They are not supported for branch or tag rulesets.
49+
//
50+
// To verify/maintain this list:
51+
// 1. Check the GitHub API documentation for organization rulesets:
52+
// https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-ruleset
53+
// 2. The API docs don't clearly separate push vs branch/tag rules. To verify,
54+
// attempt to create a branch ruleset via API or UI with each rule type.
55+
// Branch rulesets will reject push-only rules with an error.
56+
// 3. Push rules control file content: paths, sizes, extensions, path lengths.
57+
var pushOnlyRules = []string{
58+
"file_path_restriction",
59+
"max_file_path_length",
60+
"file_extension_restriction",
61+
"max_file_size",
62+
}
63+
64+
func validateRulesForTarget(ctx context.Context, d *schema.ResourceDiff) error {
65+
target := d.Get("target").(string)
66+
tflog.Debug(ctx, "Validating rules for target", map[string]any{"target": target})
67+
68+
switch target {
69+
case "push":
70+
return validateRulesForPushTarget(ctx, d)
71+
case "branch", "tag":
72+
return validateRulesForBranchTagTarget(ctx, d)
73+
}
74+
75+
tflog.Debug(ctx, "Rules validation passed", map[string]any{"target": target})
76+
return nil
77+
}
78+
79+
func validateRulesForPushTarget(ctx context.Context, d *schema.ResourceDiff) error {
80+
return validateRules(ctx, d, pushOnlyRules)
81+
}
82+
83+
func validateRulesForBranchTagTarget(ctx context.Context, d *schema.ResourceDiff) error {
84+
return validateRules(ctx, d, branchTagOnlyRules)
85+
}
86+
87+
func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []string) error {
88+
target := d.Get("target").(string)
89+
rules := d.Get("rules").([]any)[0].(map[string]any)
90+
for ruleName := range rules {
91+
ruleValue, exists := d.GetOk(fmt.Sprintf("rules.0.%s", ruleName))
92+
if !exists {
93+
continue
94+
}
95+
switch ruleValue := ruleValue.(type) {
96+
case []any:
97+
if len(ruleValue) == 0 {
98+
continue
99+
}
100+
case map[string]any:
101+
if len(ruleValue) == 0 {
102+
continue
103+
}
104+
case any:
105+
if ruleValue == nil {
106+
continue
107+
}
108+
}
109+
if slices.Contains(allowedRules, ruleName) {
110+
continue
111+
} else {
112+
tflog.Debug(ctx, fmt.Sprintf("Invalid rule for %s target", target), map[string]any{"rule": ruleName, "value": ruleValue})
113+
return fmt.Errorf("rule %q is not valid for %[2]s target; %[2]s targets only support: %v", ruleName, target, allowedRules)
114+
}
115+
}
116+
tflog.Debug(ctx, fmt.Sprintf("Rules validation passed for %s target", target))
117+
return nil
118+
}
119+
120+
func validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx context.Context, target string, conditions map[string]any) error {
121+
tflog.Debug(ctx, fmt.Sprintf("Validating conditions field for %s target", target), map[string]any{"target": target, "conditions": conditions})
122+
123+
if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 {
124+
tflog.Debug(ctx, fmt.Sprintf("Missing ref_name for %s target", target), map[string]any{"target": target})
125+
return fmt.Errorf("ref_name must be set for %s target", target)
126+
}
127+
128+
tflog.Debug(ctx, fmt.Sprintf("Conditions validation passed for %s target", target))
129+
return nil
130+
}
131+
132+
func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, target string, conditions map[string]any) error {
133+
tflog.Debug(ctx, fmt.Sprintf("Validating conditions field for %s target", target), map[string]any{"target": target, "conditions": conditions})
134+
135+
if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 {
136+
tflog.Debug(ctx, fmt.Sprintf("Missing ref_name for %s target", target), map[string]any{"target": target})
137+
return fmt.Errorf("ref_name must be set for %s target", target)
138+
}
139+
140+
if (conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0) && (conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0) {
141+
tflog.Debug(ctx, fmt.Sprintf("Missing repository_name or repository_id for %s target", target), map[string]any{"target": target})
142+
return fmt.Errorf("either repository_name or repository_id must be set for %s target", target)
143+
}
144+
tflog.Debug(ctx, fmt.Sprintf("Conditions validation passed for %s target", target))
145+
return nil
146+
}
147+
148+
func validateConditionsFieldForPushTarget(ctx context.Context, conditions map[string]any) error {
149+
tflog.Debug(ctx, "Validating conditions field for push target", map[string]any{"target": "push", "conditions": conditions})
150+
151+
if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 {
152+
tflog.Debug(ctx, "Invalid ref_name for push target", map[string]any{"ref_name": conditions["ref_name"]})
153+
return fmt.Errorf("ref_name must not be set for push target")
154+
}
155+
tflog.Debug(ctx, "Conditions validation passed for push target")
156+
return nil
157+
}
158+
159+
func validateConditionsFieldForRepositoryTarget(ctx context.Context, conditions map[string]any) error {
160+
tflog.Debug(ctx, "Validating conditions field for repository target", map[string]any{"target": "repository", "conditions": conditions})
161+
162+
if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 {
163+
tflog.Debug(ctx, "Invalid ref_name for repository target", map[string]any{"ref_name": conditions["ref_name"]})
164+
return fmt.Errorf("ref_name must not be set for repository target")
165+
}
166+
if conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0 || conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0 {
167+
return fmt.Errorf("one of repository_name or repository_id must be set for repository target")
168+
}
169+
tflog.Debug(ctx, "Conditions validation passed for repository target")
170+
return nil
171+
}

‎github/repository_rules_validation_utils_test.go‎

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
package github
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestValidateConditionsFieldForPushTarget(t *testing.T) {
8+
tests := []struct {
9+
name string
10+
conditions map[string]any
11+
expectError bool
12+
errorMsg string
13+
}{
14+
{
15+
name: "valid push target without ref_name",
16+
conditions: map[string]any{
17+
"repository_name": []any{map[string]any{"include": []any{"~ALL"}, "exclude": []any{}}},
18+
},
19+
expectError: false,
20+
},
21+
{
22+
name: "valid push target with nil ref_name",
23+
conditions: map[string]any{"ref_name": nil},
24+
expectError: false,
25+
},
26+
{
27+
name: "valid push target with empty ref_name slice",
28+
conditions: map[string]any{"ref_name": []any{}},
29+
expectError: false,
30+
},
31+
{
32+
name: "invalid push target with ref_name set",
33+
conditions: map[string]any{
34+
"ref_name": []any{map[string]any{"include": []any{"~ALL"}, "exclude": []any{}}},
35+
},
36+
expectError: true,
37+
errorMsg: "ref_name must not be set for push target",
38+
},
39+
}
40+
41+
for _, tt := range tests {
42+
t.Run(tt.name, func(t *testing.T) {
43+
err := validateConditionsFieldForPushTarget(t.Context(), tt.conditions)
44+
if tt.expectError {
45+
if err == nil {
46+
t.Errorf("expected error but got nil")
47+
} else if err.Error() != tt.errorMsg {
48+
t.Errorf("expected error %q, got %q", tt.errorMsg, err.Error())
49+
}
50+
} else {
51+
if err != nil {
52+
t.Errorf("expected no error but got: %v", err)
53+
}
54+
}
55+
})
56+
}
57+
}
58+
59+
func TestValidateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *testing.T) {
60+
tests := []struct {
61+
name string
62+
target string
63+
conditions map[string]any
64+
expectError bool
65+
errorMsg string
66+
}{
67+
{
68+
name: "valid branch target with ref_name",
69+
target: "branch",
70+
conditions: map[string]any{
71+
"ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}},
72+
},
73+
expectError: false,
74+
},
75+
{
76+
name: "valid tag target with ref_name",
77+
target: "tag",
78+
conditions: map[string]any{
79+
"ref_name": []any{map[string]any{"include": []any{"v*"}, "exclude": []any{}}},
80+
},
81+
expectError: false,
82+
},
83+
{
84+
name: "invalid branch target without ref_name",
85+
target: "branch",
86+
conditions: map[string]any{},
87+
expectError: true,
88+
errorMsg: "ref_name must be set for branch target",
89+
},
90+
{
91+
name: "invalid tag target without ref_name",
92+
target: "tag",
93+
conditions: map[string]any{},
94+
expectError: true,
95+
errorMsg: "ref_name must be set for tag target",
96+
},
97+
{
98+
name: "invalid branch target with nil ref_name",
99+
target: "branch",
100+
conditions: map[string]any{"ref_name": nil},
101+
expectError: true,
102+
errorMsg: "ref_name must be set for branch target",
103+
},
104+
{
105+
name: "invalid tag target with empty ref_name slice",
106+
target: "tag",
107+
conditions: map[string]any{"ref_name": []any{}},
108+
expectError: true,
109+
errorMsg: "ref_name must be set for tag target",
110+
},
111+
}
112+
113+
for _, tt := range tests {
114+
t.Run(tt.name, func(t *testing.T) {
115+
err := validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions)
116+
if tt.expectError {
117+
if err == nil {
118+
t.Errorf("expected error but got nil")
119+
} else if err.Error() != tt.errorMsg {
120+
t.Errorf("expected error %q, got %q", tt.errorMsg, err.Error())
121+
}
122+
} else {
123+
if err != nil {
124+
t.Errorf("expected no error but got: %v", err)
125+
}
126+
}
127+
})
128+
}
129+
}
130+
131+
func TestValidateConditionsFieldForBranchAndTagTargets(t *testing.T) {
132+
tests := []struct {
133+
name string
134+
target string
135+
conditions map[string]any
136+
expectError bool
137+
errorMsg string
138+
}{
139+
{
140+
name: "valid branch target with ref_name and repository_name",
141+
target: "branch",
142+
conditions: map[string]any{
143+
"ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}},
144+
"repository_name": []any{map[string]any{"include": []any{"~ALL"}, "exclude": []any{}}},
145+
},
146+
expectError: false,
147+
},
148+
{
149+
name: "valid tag target with ref_name and repository_id",
150+
target: "tag",
151+
conditions: map[string]any{
152+
"ref_name": []any{map[string]any{"include": []any{"v*"}, "exclude": []any{}}},
153+
"repository_id": []any{123, 456},
154+
},
155+
expectError: false,
156+
},
157+
{
158+
name: "invalid branch target without ref_name",
159+
target: "branch",
160+
conditions: map[string]any{
161+
"repository_name": []any{map[string]any{"include": []any{"~ALL"}, "exclude": []any{}}},
162+
},
163+
expectError: true,
164+
errorMsg: "ref_name must be set for branch target",
165+
},
166+
{
167+
name: "invalid branch target without repository_name or repository_id",
168+
target: "branch",
169+
conditions: map[string]any{
170+
"ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}},
171+
},
172+
expectError: true,
173+
errorMsg: "either repository_name or repository_id must be set for branch target",
174+
},
175+
{
176+
name: "invalid tag target with nil repository_name and repository_id",
177+
target: "tag",
178+
conditions: map[string]any{
179+
"ref_name": []any{map[string]any{"include": []any{"v*"}, "exclude": []any{}}},
180+
"repository_name": nil,
181+
"repository_id": nil,
182+
},
183+
expectError: true,
184+
errorMsg: "either repository_name or repository_id must be set for tag target",
185+
},
186+
{
187+
name: "invalid branch target with empty repository_name and repository_id slices",
188+
target: "branch",
189+
conditions: map[string]any{
190+
"ref_name": []any{map[string]any{"include": []any{"~DEFAULT_BRANCH"}, "exclude": []any{}}},
191+
"repository_name": []any{},
192+
"repository_id": []any{},
193+
},
194+
expectError: true,
195+
errorMsg: "either repository_name or repository_id must be set for branch target",
196+
},
197+
}
198+
199+
for _, tt := range tests {
200+
t.Run(tt.name, func(t *testing.T) {
201+
err := validateConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions)
202+
if tt.expectError {
203+
if err == nil {
204+
t.Errorf("expected error but got nil")
205+
} else if err.Error() != tt.errorMsg {
206+
t.Errorf("expected error %q, got %q", tt.errorMsg, err.Error())
207+
}
208+
} else {
209+
if err != nil {
210+
t.Errorf("expected no error but got: %v", err)
211+
}
212+
}
213+
})
214+
}
215+
}
216+
217+
func TestRuleListsDoNotOverlap(t *testing.T) {
218+
for _, pushRule := range pushOnlyRules {
219+
for _, branchTagRule := range branchTagOnlyRules {
220+
if pushRule == branchTagRule {
221+
t.Errorf("rule %q appears in both pushOnlyRules and branchTagOnlyRules", pushRule)
222+
}
223+
}
224+
}
225+
}

0 commit comments

Comments
 (0)