[MAINT] Fix github_organization_ruleset with push target
#2958
+591
−152
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,25 +6,30 @@ import ( | |
| "fmt" | ||
| "log" | ||
| "net/http" | ||
| "regexp" | ||
| "strconv" | ||
|
|
||
| "github.com/google/go-github/v77/github" | ||
| "github.com/hashicorp/terraform-plugin-log/tflog" | ||
| "github.com/hashicorp/terraform-plugin-sdk/v2/diag" | ||
| "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" | ||
| "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" | ||
| ) | ||
|
|
||
| func resourceGithubOrganizationRuleset() *schema.Resource { | ||
| return &schema.Resource{ | ||
| CreateContext: resourceGithubOrganizationRulesetCreate, | ||
| ReadContext: resourceGithubOrganizationRulesetRead, | ||
| UpdateContext: resourceGithubOrganizationRulesetUpdate, | ||
| DeleteContext: resourceGithubOrganizationRulesetDelete, | ||
| Importer: &schema.ResourceImporter{ | ||
| StateContext: resourceGithubOrganizationRulesetImport, | ||
| }, | ||
|
|
||
| SchemaVersion: 1, | ||
|
|
||
| CustomizeDiff: validateConditionsFieldBasedOnTarget, | ||
|
|
||
| Schema: map[string]*schema.Schema{ | ||
| "name": { | ||
| Type: schema.TypeString, | ||
|
|
@@ -35,8 +40,8 @@ func resourceGithubOrganizationRuleset() *schema.Resource { | |
| "target": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| ValidateFunc: validation.StringInSlice([]string{"branch", "tag", "push", "repository"}, false), | ||
| Description: "The target of the ruleset. Possible values are `branch`, `tag`, `push` and `repository`.", | ||
deiga marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }, | ||
| "enforcement": { | ||
| Type: schema.TypeString, | ||
|
|
@@ -45,7 +50,7 @@ func resourceGithubOrganizationRuleset() *schema.Resource { | |
| Description: "Possible values for Enforcement are `disabled`, `active`, `evaluate`. Note: `evaluate` is currently only supported for owners of type `organization`.", | ||
| }, | ||
| "bypass_actors": { | ||
| Type: schema.TypeList, // TODO: These are returned from GH API sorted by actor_id, we might want to investigate if we want to include sorting | ||
|
Collaborator
There was a problem hiding this comment. I think we should sort the returned values based on the inputs which would stop churn.
Contributor
Author
There was a problem hiding this comment. That sounds sensible, need to investigate where the best place for the sorting is |
||
| Optional: true, | ||
| DiffSuppressFunc: bypassActorsDiffSuppressFunc, | ||
| Description: "The actors that can bypass the rules in this ruleset.", | ||
|
|
@@ -86,12 +91,12 @@ func resourceGithubOrganizationRuleset() *schema.Resource { | |
| Type: schema.TypeList, | ||
| Optional: true, | ||
| MaxItems: 1, | ||
| Description: "Parameters for an organization ruleset condition. For `branch` and `tag` targets, `ref_name` is required alongside one of `repository_name` or `repository_id`. The `push` target does not require `ref_name` conditions. For `repository` target, the conditions object should only contain the `repository_name` and the `repository_id`.", | ||
deiga marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
| "ref_name": { | ||
| Type: schema.TypeList, | ||
| Optional: true, | ||
|
Member
There was a problem hiding this comment. This is a non breaking change ➕ |
||
| MaxItems: 1, | ||
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
|
|
@@ -197,6 +202,16 @@ func resourceGithubOrganizationRuleset() *schema.Resource { | |
| Description: "Require all commits be made to a non-target branch and submitted via a pull request before they can be merged.", | ||
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
| "allowed_merge_methods": { | ||
| Type: schema.TypeList, | ||
| Required: true, | ||
| MinItems: 1, | ||
| Description: "Array of allowed merge methods. Allowed values include `merge`, `squash`, and `rebase`. At least one option must be enabled.", | ||
| Elem: &schema.Schema{ | ||
| Type: schema.TypeString, | ||
| ValidateDiagFunc: toDiagFunc(validation.StringInSlice([]string{"merge", "squash", "rebase"}, false), "allowed_merge_methods"), | ||
| }, | ||
| }, | ||
| "dismiss_stale_reviews_on_push": { | ||
| Type: schema.TypeBool, | ||
| Optional: true, | ||
|
|
@@ -245,9 +260,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { | |
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
| "context": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| ValidateDiagFunc: validation.ToDiagFunc(validation.StringIsNotEmpty), | ||
| Description: "The status check context name that must be present on the commit.", | ||
| }, | ||
| "integration_id": { | ||
| Type: schema.TypeInt, | ||
|
|
@@ -454,9 +470,10 @@ func resourceGithubOrganizationRuleset() *schema.Resource { | |
| Description: "The repository in which the workflow is defined.", | ||
| }, | ||
| "path": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| ValidateDiagFunc: toDiagFunc(validation.StringMatch(regexp.MustCompile(`^\.github\/workflows\/.*$`), "Path must be in the .github/workflows directory"), "path"), | ||
| Description: "The path to the workflow YAML definition file.", | ||
| }, | ||
| "ref": { | ||
| Type: schema.TypeString, | ||
|
|
@@ -585,36 +602,35 @@ func resourceGithubOrganizationRuleset() *schema.Resource { | |
| } | ||
| } | ||
|
|
||
| func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { | ||
| client := meta.(*Owner).v3client | ||
|
|
||
| rulesetReq := resourceGithubRulesetObject(d, meta.(*Owner).name) | ||
|
|
||
| org := meta.(*Owner).name | ||
|
|
||
| var ruleset *github.RepositoryRuleset | ||
| var err error | ||
|
|
||
| ruleset, _, err = client.Organizations.CreateRepositoryRuleset(ctx, org, *rulesetReq) | ||
| if err != nil { | ||
| return diag.FromErr(err) | ||
| } | ||
| d.SetId(strconv.FormatInt(*ruleset.ID, 10)) | ||
|
|
||
| return resourceGithubOrganizationRulesetRead(ctx, d, meta) | ||
| } | ||
|
|
||
| func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { | ||
| client := meta.(*Owner).v3client | ||
|
|
||
| org := meta.(*Owner).name | ||
| rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) | ||
| if err != nil { | ||
| return diag.FromErr(unconvertibleIdErr(d.Id(), err)) | ||
| } | ||
|
|
||
| ctx = context.WithValue(ctx, ctxId, rulesetID) | ||
| if !d.IsNewResource() { | ||
| ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string)) | ||
| } | ||
|
|
@@ -636,7 +652,7 @@ func resourceGithubOrganizationRulesetRead(d *schema.ResourceData, meta any) err | |
| return nil | ||
| } | ||
| } | ||
| return diag.FromErr(err) | ||
| } | ||
|
|
||
| if ruleset == nil { | ||
|
|
@@ -659,46 +675,46 @@ func resourceGithubOrganizationRulesetRead(d *schema.ResourceData, meta any) err | |
| return nil | ||
| } | ||
|
|
||
| func resourceGithubOrganizationRulesetUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { | ||
| client := meta.(*Owner).v3client | ||
|
|
||
| rulesetReq := resourceGithubRulesetObject(d, meta.(*Owner).name) | ||
|
|
||
| org := meta.(*Owner).name | ||
| rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) | ||
| if err != nil { | ||
| return diag.FromErr(unconvertibleIdErr(d.Id(), err)) | ||
| } | ||
|
|
||
| ctx = context.WithValue(ctx, ctxId, rulesetID) | ||
|
|
||
| var ruleset *github.RepositoryRuleset | ||
|
|
||
| ruleset, _, err = client.Organizations.UpdateRepositoryRuleset(ctx, org, rulesetID, *rulesetReq) | ||
|
Collaborator
There was a problem hiding this comment. Checking if the bypass actors have changed and the length is |
||
| if err != nil { | ||
| return diag.FromErr(err) | ||
| } | ||
| d.SetId(strconv.FormatInt(*ruleset.ID, 10)) | ||
|
|
||
| return resourceGithubOrganizationRulesetRead(ctx, d, meta) | ||
| } | ||
|
|
||
| func resourceGithubOrganizationRulesetDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { | ||
| client := meta.(*Owner).v3client | ||
| org := meta.(*Owner).name | ||
|
|
||
| rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) | ||
| if err != nil { | ||
| return diag.FromErr(unconvertibleIdErr(d.Id(), err)) | ||
| } | ||
| ctx = context.WithValue(ctx, ctxId, rulesetID) | ||
|
|
||
| log.Printf("[DEBUG] Deleting organization ruleset: %s: %d", org, rulesetID) | ||
| _, err = client.Organizations.DeleteRepositoryRuleset(ctx, org, rulesetID) | ||
| return diag.FromErr(err) | ||
| } | ||
|
|
||
| func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { | ||
| rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) | ||
| if err != nil { | ||
| return []*schema.ResourceData{d}, unconvertibleIdErr(d.Id(), err) | ||
|
|
@@ -710,7 +726,6 @@ func resourceGithubOrganizationRulesetImport(d *schema.ResourceData, meta any) ( | |
|
|
||
| client := meta.(*Owner).v3client | ||
| org := meta.(*Owner).name | ||
|
|
||
| ruleset, _, err := client.Organizations.GetRepositoryRuleset(ctx, org, rulesetID) | ||
| if ruleset == nil || err != nil { | ||
|
|
@@ -720,3 +735,62 @@ func resourceGithubOrganizationRulesetImport(d *schema.ResourceData, meta any) ( | |
|
|
||
| return []*schema.ResourceData{d}, nil | ||
| } | ||
|
|
||
| func validateConditionsFieldForBranchAndTagTargets(ctx context.Context, d *schema.ResourceDiff, _ any) error { | ||
| target := d.Get("target").(string) | ||
| conditions := d.Get("conditions").([]any)[0].(map[string]any) | ||
| tflog.Debug(ctx, "Validating conditions field for branch and tag targets", map[string]any{"target": target, "conditions": conditions}) | ||
| if conditions["ref_name"] == nil || len(conditions["ref_name"].([]any)) == 0 { | ||
| 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) { | ||
| return fmt.Errorf("either repository_name or repository_id must be set for %s target", target) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func validateConditionsFieldForPushTarget(ctx context.Context, d *schema.ResourceDiff, _ any) error { | ||
| target := d.Get("target").(string) | ||
| conditions := d.Get("conditions").([]any)[0].(map[string]any) | ||
| tflog.Debug(ctx, "Validating conditions field for push target", map[string]any{"target": target, "conditions": conditions}) | ||
|
|
||
| if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { | ||
| return fmt.Errorf("ref_name must not be set for %s target", target) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func validateConditionsFieldForRepositoryTarget(ctx context.Context, d *schema.ResourceDiff, _ any) error { | ||
| target := d.Get("target").(string) | ||
| conditions := d.Get("conditions").([]any)[0].(map[string]any) | ||
| tflog.Debug(ctx, "Validating conditions field for repository target", map[string]any{"target": target, "conditions": conditions}) | ||
|
|
||
| if conditions["ref_name"] != nil && len(conditions["ref_name"].([]any)) > 0 { | ||
| return fmt.Errorf("ref_name must not 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 { | ||
| return fmt.Errorf("repository_name or repository_id must be set for %s target", target) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func validateConditionsFieldBasedOnTarget(ctx context.Context, d *schema.ResourceDiff, meta any) error { | ||
| target := d.Get("target").(string) | ||
| tflog.Debug(ctx, "Validating conditions field based on target", map[string]any{"target": target}) | ||
| conditionsRaw := d.Get("conditions").([]any) | ||
|
|
||
| if len(conditionsRaw) == 0 { | ||
| tflog.Debug(ctx, "An empty conditions block, skipping validation.", map[string]any{"target": target}) | ||
| return nil | ||
| } | ||
|
|
||
| switch target { | ||
| case "branch", "tag": | ||
| return validateConditionsFieldForBranchAndTagTargets(ctx, d, meta) | ||
| case "push": | ||
| return validateConditionsFieldForPushTarget(ctx, d, meta) | ||
| case "repository": | ||
| return validateConditionsFieldForRepositoryTarget(ctx, d, meta) | ||
| } | ||
| return nil | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain why we need this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't inititialize the
GitHubsubsystem anywhere and from my understanding subsystems are usually per-resource things.With an unitialized Subsystem the logs get spammed with warnings about it.
But this implementation is "actually" in this PR #2976 and any modifications to it should happen there