Skip to content

Commit b276c34

Browse files
deigadeiga
committed
Refactor validation functions so that Repo and Org share almost everything
Signed-off-by: Timo Sand <[email protected]>
1 parent 89f2f17 commit b276c34

4 files changed

Lines changed: 56 additions & 80 deletions

‎github/resource_github_organization_ruleset.go‎

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/google/go-github/v81/github"
1212
"github.com/hashicorp/terraform-plugin-log/tflog"
1313
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
14-
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
1514
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1615
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
1716
)
@@ -28,10 +27,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource {
2827

2928
SchemaVersion: 1,
3029

31-
CustomizeDiff: customdiff.All(
32-
validateConditionsFieldBasedOnTarget,
33-
validateOrganizationRulesetRules,
34-
),
30+
CustomizeDiff: resourceGithubOrganizationRulesetValidate,
3531

3632
Schema: map[string]*schema.Schema{
3733
"name": {
@@ -867,36 +863,16 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso
867863
return []*schema.ResourceData{d}, nil
868864
}
869865

870-
func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error {
871-
target := Target(d.Get("target").(string))
872-
tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target})
873-
conditionsRaw := d.Get("conditions").([]any)
874-
875-
if len(conditionsRaw) == 0 {
876-
tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]any{"target": target})
877-
return nil
878-
}
879-
880-
conditions := conditionsRaw[0].(map[string]any)
881-
882-
switch target {
883-
case TargetBranch, TargetTag:
884-
return validateConditionsFieldForBranchAndTagTargets(ctx, target, conditions)
885-
case TargetPush:
886-
return validateConditionsFieldForPushTarget(ctx, conditions)
866+
func resourceGithubOrganizationRulesetValidate(ctx context.Context, d *schema.ResourceDiff, _ any) error {
867+
err := validateRulesetConditions(ctx, d, true)
868+
if err != nil {
869+
return err
887870
}
888-
return nil
889-
}
890-
891-
func validateOrganizationRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error {
892-
target := Target(d.Get("target").(string))
893-
tflog.Debug(ctx, "Validating organization ruleset rules based on target", map[string]any{"target": target})
894871

895-
rulesRaw := d.Get("rules").([]any)
896-
if len(rulesRaw) == 0 {
897-
tflog.Debug(ctx, "No rules block, skipping validation")
898-
return nil
872+
err = validateRulesetRules(ctx, d)
873+
if err != nil {
874+
return err
899875
}
900876

901-
return validateRulesForTarget(ctx, d)
877+
return nil
902878
}

‎github/resource_github_repository_ruleset.go‎

Lines changed: 9 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"github.com/google/go-github/v81/github"
1111
"github.com/hashicorp/terraform-plugin-log/tflog"
1212
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
13-
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
1413
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1514
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
1615
)
@@ -27,10 +26,7 @@ func resourceGithubRepositoryRuleset() *schema.Resource {
2726

2827
SchemaVersion: 1,
2928

30-
CustomizeDiff: customdiff.All(
31-
validateRepositoryRulesetConditions,
32-
validateRepositoryRulesetRules,
33-
),
29+
CustomizeDiff: resourceGithubRepositoryRulesetValidate,
3430

3531
Schema: map[string]*schema.Schema{
3632
"name": {
@@ -796,37 +792,16 @@ func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.Resour
796792
return []*schema.ResourceData{d}, nil
797793
}
798794

799-
// validateRepositoryRulesetConditions validates conditions based on target type.
800-
func validateRepositoryRulesetConditions(ctx context.Context, d *schema.ResourceDiff, _ any) error {
801-
target := d.Get("target").(string)
802-
tflog.Debug(ctx, "Validating repository ruleset conditions", map[string]any{"target": target})
803-
804-
conditionsRaw := d.Get("conditions").([]any)
805-
if len(conditionsRaw) == 0 {
806-
tflog.Debug(ctx, "No conditions block, skipping validation")
807-
return nil
808-
}
809-
810-
conditions := conditionsRaw[0].(map[string]any)
811-
812-
switch Target(target) {
813-
case TargetBranch, TargetTag:
814-
return validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx, Target(target), conditions)
815-
case TargetPush:
816-
return validateConditionsFieldForPushTarget(ctx, conditions)
795+
func resourceGithubRepositoryRulesetValidate(ctx context.Context, d *schema.ResourceDiff, meta any) error {
796+
err := validateRulesetConditions(ctx, d, false)
797+
if err != nil {
798+
return err
817799
}
818-
return nil
819-
}
820-
821-
func validateRepositoryRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error {
822-
target := d.Get("target").(string)
823-
tflog.Debug(ctx, "Validating repository ruleset rules based on target", map[string]any{"target": target})
824800

825-
rulesRaw := d.Get("rules").([]any)
826-
if len(rulesRaw) == 0 {
827-
tflog.Debug(ctx, "No rules block, skipping validation")
828-
return nil
801+
err = validateRulesetRules(ctx, d)
802+
if err != nil {
803+
return err
829804
}
830805

831-
return validateRulesForTarget(ctx, d)
806+
return nil
832807
}

‎github/util_ruleset_validation.go‎

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -117,29 +117,54 @@ func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []s
117117
return nil
118118
}
119119

120-
func validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx context.Context, target Target, 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})
120+
func validateRulesetConditions(ctx context.Context, d *schema.ResourceDiff, isOrg bool) error {
121+
target := Target(d.Get("target").(string))
122+
tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target})
123+
conditionsRaw := d.Get("conditions").([]any)
122124

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)
125+
if len(conditionsRaw) == 0 {
126+
tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]any{"target": target})
127+
return nil
126128
}
127129

128-
tflog.Debug(ctx, fmt.Sprintf("Conditions validation passed for %s target", target))
130+
conditions := conditionsRaw[0].(map[string]any)
131+
132+
switch target {
133+
case TargetBranch, TargetTag:
134+
return validateConditionsFieldForBranchAndTagTargets(ctx, target, conditions, isOrg)
135+
case TargetPush:
136+
return validateConditionsFieldForPushTarget(ctx, conditions)
137+
}
129138
return nil
130139
}
131140

132-
func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, target Target, 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})
141+
func validateRulesetRules(ctx context.Context, d *schema.ResourceDiff) error {
142+
target := Target(d.Get("target").(string))
143+
tflog.Debug(ctx, "Validating ruleset rules based on target", map[string]any{"target": target})
144+
145+
rulesRaw := d.Get("rules").([]any)
146+
if len(rulesRaw) == 0 {
147+
tflog.Debug(ctx, "No rules block, skipping validation")
148+
return nil
149+
}
150+
151+
return validateRulesForTarget(ctx, d)
152+
}
153+
154+
func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, target Target, conditions map[string]any, isOrg bool) error {
155+
tflog.Debug(ctx, fmt.Sprintf("Validating conditions field for %s target", target), map[string]any{"target": target, "conditions": conditions, "isOrg": isOrg})
134156

135157
if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 {
136158
tflog.Debug(ctx, fmt.Sprintf("Missing ref_name for %s target", target), map[string]any{"target": target})
137159
return fmt.Errorf("ref_name must be set for %s target", target)
138160
}
139161

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)
162+
// Repository rulesets don't have repository_name or repository_id, only org rulesets do.
163+
if isOrg {
164+
if (conditions["repository_name"] == nil || len(conditions["repository_name"].([]any)) == 0) && (conditions["repository_id"] == nil || len(conditions["repository_id"].([]any)) == 0) {
165+
tflog.Debug(ctx, fmt.Sprintf("Missing repository_name or repository_id for %s target", target), map[string]any{"target": target})
166+
return fmt.Errorf("either repository_name or repository_id must be set for %s target", target)
167+
}
143168
}
144169
tflog.Debug(ctx, fmt.Sprintf("Conditions validation passed for %s target", target))
145170
return nil

‎github/util_ruleset_validation_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func Test_validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t *test
112112

113113
for _, tt := range tests {
114114
t.Run(tt.name, func(t *testing.T) {
115-
err := validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions)
115+
err := validateConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions, false)
116116
if tt.expectError {
117117
if err == nil {
118118
t.Errorf("expected error but got nil")
@@ -198,7 +198,7 @@ func Test_validateConditionsFieldForBranchAndTagTargets(t *testing.T) {
198198

199199
for _, tt := range tests {
200200
t.Run(tt.name, func(t *testing.T) {
201-
err := validateConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions)
201+
err := validateConditionsFieldForBranchAndTagTargets(t.Context(), tt.target, tt.conditions, true)
202202
if tt.expectError {
203203
if err == nil {
204204
t.Errorf("expected error but got nil")

0 commit comments

Comments
 (0)