Skip to content

Commit c8d93e0

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

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
@@ -118,29 +118,54 @@ func validateRules(ctx context.Context, d *schema.ResourceDiff, allowedRules []s
118118
return nil
119119
}
120120

121-
func validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx context.Context, target Target, conditions map[string]any) error {
122-
tflog.Debug(ctx, fmt.Sprintf("Validating conditions field for %s target", target), map[string]any{"target": target, "conditions": conditions})
121+
func validateRulesetConditions(ctx context.Context, d *schema.ResourceDiff, isOrg bool) error {
122+
target := Target(d.Get("target").(string))
123+
tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target})
124+
conditionsRaw := d.Get("conditions").([]any)
123125

124-
if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 {
125-
tflog.Debug(ctx, fmt.Sprintf("Missing ref_name for %s target", target), map[string]any{"target": target})
126-
return fmt.Errorf("ref_name must be set for %s target", target)
126+
if len(conditionsRaw) == 0 {
127+
tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]any{"target": target})
128+
return nil
127129
}
128130

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

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

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

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