Skip to content

Commit 5c0112b

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

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": {
@@ -845,36 +841,16 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso
845841
return []*schema.ResourceData{d}, nil
846842
}
847843

848-
func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error {
849-
target := Target(d.Get("target").(string))
850-
tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target})
851-
conditionsRaw := d.Get("conditions").([]any)
852-
853-
if len(conditionsRaw) == 0 {
854-
tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]any{"target": target})
855-
return nil
856-
}
857-
858-
conditions := conditionsRaw[0].(map[string]any)
859-
860-
switch target {
861-
case TargetBranch, TargetTag:
862-
return validateConditionsFieldForBranchAndTagTargets(ctx, target, conditions)
863-
case TargetPush:
864-
return validateConditionsFieldForPushTarget(ctx, conditions)
844+
func resourceGithubOrganizationRulesetValidate(ctx context.Context, d *schema.ResourceDiff, _ any) error {
845+
err := validateRulesetConditions(ctx, d, true)
846+
if err != nil {
847+
return err
865848
}
866-
return nil
867-
}
868-
869-
func validateOrganizationRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error {
870-
target := Target(d.Get("target").(string))
871-
tflog.Debug(ctx, "Validating organization ruleset rules based on target", map[string]any{"target": target})
872849

873-
rulesRaw := d.Get("rules").([]any)
874-
if len(rulesRaw) == 0 {
875-
tflog.Debug(ctx, "No rules block, skipping validation")
876-
return nil
850+
err = validateRulesetRules(ctx, d)
851+
if err != nil {
852+
return err
877853
}
878854

879-
return validateRulesForTarget(ctx, d)
855+
return nil
880856
}

‎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": {
@@ -774,37 +770,16 @@ func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.Resour
774770
return []*schema.ResourceData{d}, nil
775771
}
776772

777-
// validateRepositoryRulesetConditions validates conditions based on target type.
778-
func validateRepositoryRulesetConditions(ctx context.Context, d *schema.ResourceDiff, _ any) error {
779-
target := d.Get("target").(string)
780-
tflog.Debug(ctx, "Validating repository ruleset conditions", map[string]any{"target": target})
781-
782-
conditionsRaw := d.Get("conditions").([]any)
783-
if len(conditionsRaw) == 0 {
784-
tflog.Debug(ctx, "No conditions block, skipping validation")
785-
return nil
786-
}
787-
788-
conditions := conditionsRaw[0].(map[string]any)
789-
790-
switch Target(target) {
791-
case TargetBranch, TargetTag:
792-
return validateRepositoryRulesetConditionsFieldForBranchAndTagTargets(ctx, Target(target), conditions)
793-
case TargetPush:
794-
return validateConditionsFieldForPushTarget(ctx, conditions)
773+
func resourceGithubRepositoryRulesetValidate(ctx context.Context, d *schema.ResourceDiff, meta any) error {
774+
err := validateRulesetConditions(ctx, d, false)
775+
if err != nil {
776+
return err
795777
}
796-
return nil
797-
}
798-
799-
func validateRepositoryRulesetRules(ctx context.Context, d *schema.ResourceDiff, _ any) error {
800-
target := d.Get("target").(string)
801-
tflog.Debug(ctx, "Validating repository ruleset rules based on target", map[string]any{"target": target})
802778

803-
rulesRaw := d.Get("rules").([]any)
804-
if len(rulesRaw) == 0 {
805-
tflog.Debug(ctx, "No rules block, skipping validation")
806-
return nil
779+
err = validateRulesetRules(ctx, d)
780+
if err != nil {
781+
return err
807782
}
808783

809-
return validateRulesForTarget(ctx, d)
784+
return nil
810785
}

‎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)