diff --git a/github/config.go b/github/config.go index d5f561c19f..de232c1766 100644 --- a/github/config.go +++ b/github/config.go @@ -59,7 +59,7 @@ var ( func RateLimitedHTTPClient(client *http.Client, writeDelay, readDelay, retryDelay time.Duration, parallelRequests bool, retryableErrors map[int]bool, maxRetries int) *http.Client { client.Transport = NewEtagTransport(client.Transport) client.Transport = NewRateLimitTransport(client.Transport, WithWriteDelay(writeDelay), WithReadDelay(readDelay), WithParallelRequests(parallelRequests)) - client.Transport = logging.NewSubsystemLoggingHTTPTransport("GitHub", client.Transport) + client.Transport = logging.NewLoggingHTTPTransport(client.Transport) client.Transport = newPreviewHeaderInjectorTransport(map[string]string{ // TODO: remove when Stone Crop preview is moved to general availability in the GraphQL API "Accept": "application/vnd.github.stone-crop-preview+json", diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index 46cca07781..eea93ffc0a 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -4,11 +4,11 @@ import ( "context" "errors" "fmt" - "log" "net/http" "strconv" "github.com/google/go-github/v81/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" @@ -46,7 +46,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, + 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 Optional: true, DiffSuppressFunc: bypassActorsDiffSuppressFunc, Description: "The actors that can bypass the rules in this ruleset.", @@ -198,6 +198,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, @@ -588,13 +598,23 @@ func resourceGithubOrganizationRuleset() *schema.Resource { func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client - owner := meta.(*Owner).name + name := d.Get("name").(string) + + tflog.Debug(ctx, fmt.Sprintf("Creating organization ruleset: %s/%s", owner, name), map[string]any{ + "owner": owner, + "name": name, + }) rulesetReq := resourceGithubRulesetObject(d, owner) ruleset, resp, err := client.Organizations.CreateRepositoryRuleset(ctx, owner, rulesetReq) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Failed to create organization ruleset: %s/%s", owner, name), map[string]any{ + "owner": owner, + "name": name, + "error": err.Error(), + }) return diag.FromErr(err) } @@ -603,16 +623,31 @@ func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.Reso _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("etag", resp.Header.Get("ETag")) + tflog.Info(ctx, fmt.Sprintf("Created organization ruleset: %s/%s (ID: %d)", owner, name, *ruleset.ID), map[string]any{ + "owner": owner, + "name": name, + "ruleset_id": *ruleset.ID, + }) + return nil } func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client - owner := meta.(*Owner).name + tflog.Trace(ctx, fmt.Sprintf("Reading organization ruleset: %s", d.Id()), map[string]any{ + "owner": owner, + "ruleset_id": d.Id(), + }) + rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Could not convert ruleset ID '%s' to int64", d.Id()), map[string]any{ + "owner": owner, + "ruleset_id": d.Id(), + "error": err.Error(), + }) return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } @@ -625,15 +660,26 @@ func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.Resour var ghErr *github.ErrorResponse if errors.As(err, &ghErr) { if ghErr.Response.StatusCode == http.StatusNotModified { + tflog.Debug(ctx, "API responded with StatusNotModified, not refreshing state", map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) return nil } if ghErr.Response.StatusCode == http.StatusNotFound { - log.Printf("[INFO] Removing ruleset %s: %d from state because it no longer exists in GitHub", - owner, rulesetID) + tflog.Info(ctx, fmt.Sprintf("Removing ruleset %s/%d from state because it no longer exists in GitHub", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) d.SetId("") return nil } } + tflog.Error(ctx, fmt.Sprintf("Failed to read organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "error": err.Error(), + }) return diag.FromErr(err) } @@ -647,23 +693,45 @@ func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.Resour _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("etag", resp.Header.Get("ETag")) + tflog.Trace(ctx, fmt.Sprintf("Successfully read organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "name": ruleset.Name, + }) + return nil } func resourceGithubOrganizationRulesetUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client - owner := meta.(*Owner).name - - rulesetReq := resourceGithubRulesetObject(d, owner) + name := d.Get("name").(string) rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Could not convert ruleset ID '%s' to int64", d.Id()), map[string]any{ + "owner": owner, + "ruleset_id": d.Id(), + "error": err.Error(), + }) return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } + tflog.Debug(ctx, fmt.Sprintf("Updating organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "name": name, + }) + + rulesetReq := resourceGithubRulesetObject(d, owner) + ruleset, resp, err := client.Organizations.UpdateRepositoryRuleset(ctx, owner, rulesetID, rulesetReq) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Failed to update organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "error": err.Error(), + }) return diag.FromErr(err) } @@ -672,6 +740,12 @@ func resourceGithubOrganizationRulesetUpdate(ctx context.Context, d *schema.Reso _ = d.Set("node_id", ruleset.GetNodeID()) _ = d.Set("etag", resp.Header.Get("ETag")) + tflog.Info(ctx, fmt.Sprintf("Updated organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "name": name, + }) + return nil } @@ -681,36 +755,79 @@ func resourceGithubOrganizationRulesetDelete(ctx context.Context, d *schema.Reso rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Could not convert ruleset ID '%s' to int64", d.Id()), map[string]any{ + "owner": owner, + "ruleset_id": d.Id(), + "error": err.Error(), + }) return diag.FromErr(unconvertibleIdErr(d.Id(), err)) } - log.Printf("[DEBUG] Deleting organization ruleset: %s: %d", owner, rulesetID) + tflog.Debug(ctx, fmt.Sprintf("Deleting organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) + _, err = client.Organizations.DeleteRepositoryRuleset(ctx, owner, rulesetID) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Failed to delete organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "error": err.Error(), + }) return diag.FromErr(err) } + tflog.Info(ctx, fmt.Sprintf("Deleted organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) + return nil } func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { + client := meta.(*Owner).v3client + owner := meta.(*Owner).name + rulesetID, err := strconv.ParseInt(d.Id(), 10, 64) if err != nil { + tflog.Error(ctx, fmt.Sprintf("Could not convert ruleset ID '%s' to int64", d.Id()), map[string]any{ + "owner": owner, + "ruleset_id": d.Id(), + "error": err.Error(), + }) return []*schema.ResourceData{d}, unconvertibleIdErr(d.Id(), err) } if rulesetID == 0 { + tflog.Error(ctx, "ruleset_id must be present and non-zero", map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) return []*schema.ResourceData{d}, fmt.Errorf("`ruleset_id` must be present") } - log.Printf("[DEBUG] Importing organization ruleset with ID: %d", rulesetID) - client := meta.(*Owner).v3client - owner := meta.(*Owner).name + tflog.Debug(ctx, fmt.Sprintf("Importing organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + }) ruleset, _, err := client.Organizations.GetRepositoryRuleset(ctx, owner, rulesetID) if ruleset == nil || err != nil { + tflog.Error(ctx, fmt.Sprintf("Failed to import organization ruleset: %s/%d", owner, rulesetID), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "error": err.Error(), + }) return []*schema.ResourceData{d}, err } d.SetId(strconv.FormatInt(ruleset.GetID(), 10)) + tflog.Info(ctx, fmt.Sprintf("Imported organization ruleset: %s/%d (name: %s)", owner, rulesetID, ruleset.Name), map[string]any{ + "owner": owner, + "ruleset_id": rulesetID, + "name": ruleset.Name, + }) + return []*schema.ResourceData{d}, nil } diff --git a/github/resource_github_organization_ruleset_test.go b/github/resource_github_organization_ruleset_test.go index 9ebd68c4bc..774315e6db 100644 --- a/github/resource_github_organization_ruleset_test.go +++ b/github/resource_github_organization_ruleset_test.go @@ -12,10 +12,46 @@ import ( func TestAccGithubOrganizationRuleset(t *testing.T) { t.Run("create_branch_ruleset", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + repoName := fmt.Sprintf("%srepo-org-ruleset-%s", testResourcePrefix, randomID) + rulesetName := fmt.Sprintf("%s-branch-ruleset-%s", testResourcePrefix, randomID) + + workflowFilePath := ".github/workflows/echo.yaml" config := fmt.Sprintf(` +resource "github_repository" "test" { + name = "%s" + visibility = "private" + auto_init = true + ignore_vulnerability_alerts_during_read = true +} + +resource "github_repository_file" "workflow_file" { + repository = github_repository.test.name + branch = "main" + file = "%[3]s" + content = < 0 @@ -115,9 +134,10 @@ func flattenBypassActors(bypassActors []*github.BypassActor) []any { actorsSlice := make([]any, 0) for _, v := range bypassActors { actorMap := make(map[string]any) - - actorMap["actor_id"] = v.GetActorID() - actorMap["actor_type"] = v.GetActorType() + actorID := v.GetActorID() + actorType := v.GetActorType() + actorMap["actor_id"] = actorID + actorMap["actor_type"] = actorType actorMap["bypass_mode"] = v.GetBypassMode() actorsSlice = append(actorsSlice, actorMap) @@ -295,13 +315,15 @@ func expandRules(input []any, org bool) *github.RepositoryRulesetRules { // Pull request rule if v, ok := rulesMap["pull_request"].([]any); ok && len(v) != 0 { pullRequestMap := v[0].(map[string]any) + allowedMergeMethods := pullRequestMap["allowed_merge_methods"] + params := &github.PullRequestRuleParameters{ - AllowedMergeMethods: []github.PullRequestMergeMethod{github.PullRequestMergeMethodMerge, github.PullRequestMergeMethodSquash, github.PullRequestMergeMethodRebase}, DismissStaleReviewsOnPush: pullRequestMap["dismiss_stale_reviews_on_push"].(bool), RequireCodeOwnerReview: pullRequestMap["require_code_owner_review"].(bool), RequireLastPushApproval: pullRequestMap["require_last_push_approval"].(bool), RequiredApprovingReviewCount: toInt(pullRequestMap["required_approving_review_count"]), RequiredReviewThreadResolution: pullRequestMap["required_review_thread_resolution"].(bool), + AllowedMergeMethods: toPullRequestMergeMethods(allowedMergeMethods), } rulesetRules.PullRequest = params } @@ -516,10 +538,14 @@ func flattenRules(rules *github.RepositoryRulesetRules, org bool) []any { // Update rule with parameters if rules.Update != nil { rulesMap["update"] = true - rulesMap["update_allows_fetch_and_merge"] = rules.Update.UpdateAllowsFetchAndMerge + if !org { + rulesMap["update_allows_fetch_and_merge"] = rules.Update.UpdateAllowsFetchAndMerge + } } else { rulesMap["update"] = false - rulesMap["update_allows_fetch_and_merge"] = false + if !org { + rulesMap["update_allows_fetch_and_merge"] = false + } } // Required deployments rule if rules.RequiredDeployments != nil { requiredDeploymentsSlice := make([]map[string]any, 0) @@ -538,7 +564,9 @@ func flattenRules(rules *github.RepositoryRulesetRules, org bool) []any { "require_last_push_approval": rules.PullRequest.RequireLastPushApproval, "required_approving_review_count": rules.PullRequest.RequiredApprovingReviewCount, "required_review_thread_resolution": rules.PullRequest.RequiredReviewThreadResolution, + "allowed_merge_methods": rules.PullRequest.AllowedMergeMethods, }) + log.Printf("[DEBUG] Flattened Pull Request rules slice request slice: %#v", pullRequestSlice) rulesMap["pull_request"] = pullRequestSlice }