diff --git a/github/resource_github_organization_ruleset.go b/github/resource_github_organization_ruleset.go index cfc67a95c7..9d67153735 100644 --- a/github/resource_github_organization_ruleset.go +++ b/github/resource_github_organization_ruleset.go @@ -768,7 +768,7 @@ func resourceGithubOrganizationRulesetCreate(ctx context.Context, d *schema.Reso rulesetReq := resourceGithubRulesetObject(d, owner) - ruleset, resp, err := client.Organizations.CreateRepositoryRuleset(ctx, owner, rulesetReq) + ruleset, resp, err := createOrgRuleset(ctx, client, owner, rulesetReq) if err != nil { tflog.Error(ctx, fmt.Sprintf("Failed to create organization ruleset: %s/%s", owner, name), map[string]any{ "owner": owner, @@ -824,7 +824,7 @@ func resourceGithubOrganizationRulesetRead(ctx context.Context, d *schema.Resour ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string)) } - ruleset, resp, err := client.Organizations.GetRepositoryRuleset(ctx, owner, rulesetID) + ruleset, resp, err := getOrgRuleset(ctx, client, owner, rulesetID) if err != nil { var ghErr *github.ErrorResponse if errors.As(err, &ghErr) { @@ -912,7 +912,7 @@ func resourceGithubOrganizationRulesetUpdate(ctx context.Context, d *schema.Reso rulesetReq := resourceGithubRulesetObject(d, owner) - ruleset, resp, err := client.Organizations.UpdateRepositoryRuleset(ctx, owner, rulesetID, rulesetReq) + ruleset, resp, err := updateOrgRuleset(ctx, client, 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, @@ -1005,7 +1005,7 @@ func resourceGithubOrganizationRulesetImport(ctx context.Context, d *schema.Reso "ruleset_id": rulesetID, }) - ruleset, _, err := client.Organizations.GetRepositoryRuleset(ctx, owner, rulesetID) + ruleset, _, err := getOrgRuleset(ctx, client, 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, diff --git a/github/resource_github_repository_ruleset.go b/github/resource_github_repository_ruleset.go index fb2549296b..651cda6c3f 100644 --- a/github/resource_github_repository_ruleset.go +++ b/github/resource_github_repository_ruleset.go @@ -702,7 +702,7 @@ func resourceGithubRepositoryRulesetCreate(ctx context.Context, d *schema.Resour return diag.Errorf("cannot create ruleset on archived repository %s/%s", owner, repoName) } - ruleset, resp, err := client.Repositories.CreateRuleset(ctx, owner, repoName, rulesetReq) + ruleset, resp, err := createRepoRuleset(ctx, client, owner, repoName, rulesetReq) if err != nil { return diag.FromErr(err) } @@ -739,7 +739,7 @@ func resourceGithubRepositoryRulesetRead(ctx context.Context, d *schema.Resource ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string)) } - ruleset, resp, err := client.Repositories.GetRuleset(ctx, owner, repoName, rulesetID, false) + ruleset, resp, err := getRepoRuleset(ctx, client, owner, repoName, rulesetID, false) if err != nil { var ghErr *github.ErrorResponse if errors.As(err, &ghErr) { @@ -815,7 +815,7 @@ func resourceGithubRepositoryRulesetUpdate(ctx context.Context, d *schema.Resour return nil } - ruleset, resp, err := client.Repositories.UpdateRuleset(ctx, owner, repoName, rulesetID, rulesetReq) + ruleset, resp, err := updateRepoRuleset(ctx, client, owner, repoName, rulesetID, rulesetReq) if err != nil { return diag.FromErr(err) } @@ -874,7 +874,7 @@ func resourceGithubRepositoryRulesetImport(ctx context.Context, d *schema.Resour return []*schema.ResourceData{d}, err } - ruleset, _, err := client.Repositories.GetRuleset(ctx, owner, repository.GetName(), rulesetID, false) + ruleset, _, err := getRepoRuleset(ctx, client, owner, repository.GetName(), rulesetID, false) if ruleset == nil || err != nil { return []*schema.ResourceData{d}, err } diff --git a/github/util_rules.go b/github/util_rules.go index 36075831f2..d7c70e744a 100644 --- a/github/util_rules.go +++ b/github/util_rules.go @@ -2,7 +2,12 @@ package github import ( "context" + "encoding/json" + "fmt" + "io" + "net/http" "reflect" + "regexp" "sort" "github.com/google/go-github/v84/github" @@ -880,6 +885,110 @@ func flattenRules(ctx context.Context, rules *github.RepositoryRulesetRules, org return []any{rulesMap} } +// reviewerStringIDPattern matches JSON "id" fields with quoted integer values. +// The GitHub API returns reviewer.id as a JSON string (e.g., "id": "12345") but +// go-github's RulesetReviewer.ID expects *int64, causing unmarshal failures. +// See https://github.com/integrations/terraform-provider-github/issues/3340 +var reviewerStringIDPattern = regexp.MustCompile(`"id"\s*:\s*"(\d+)"`) + +// fixReviewerStringIDs converts quoted integer "id" fields in raw JSON to +// unquoted integers so that go-github can unmarshal them into *int64 fields. +func fixReviewerStringIDs(data []byte) []byte { + return reviewerStringIDPattern.ReplaceAll(data, []byte(`"id":$1`)) +} + +// doRulesetRequest executes an HTTP request against the GitHub API and +// unmarshals the response into a RepositoryRuleset. It applies +// fixReviewerStringIDs to the response body before unmarshaling to work around +// the GitHub API returning reviewer.id as a string instead of a number. +func doRulesetRequest(ctx context.Context, client *github.Client, req *http.Request) (*github.RepositoryRuleset, *github.Response, error) { + resp, err := client.BareDo(ctx, req) + if err != nil { + return nil, resp, err + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, resp, err + } + + body = fixReviewerStringIDs(body) + + var rs *github.RepositoryRuleset + if err := json.Unmarshal(body, &rs); err != nil { + return nil, resp, err + } + + return rs, resp, nil +} + +// createRepoRuleset creates a repository ruleset, working around the +// reviewer.id string marshaling bug in go-github. +func createRepoRuleset(ctx context.Context, client *github.Client, owner, repo string, ruleset github.RepositoryRuleset) (*github.RepositoryRuleset, *github.Response, error) { + u := fmt.Sprintf("repos/%v/%v/rulesets", owner, repo) + req, err := client.NewRequest("POST", u, ruleset) + if err != nil { + return nil, nil, err + } + return doRulesetRequest(ctx, client, req) +} + +// getRepoRuleset gets a repository ruleset, working around the +// reviewer.id string marshaling bug in go-github. +func getRepoRuleset(ctx context.Context, client *github.Client, owner, repo string, rulesetID int64, includesParents bool) (*github.RepositoryRuleset, *github.Response, error) { + u := fmt.Sprintf("repos/%v/%v/rulesets/%v?includes_parents=%v", owner, repo, rulesetID, includesParents) + req, err := client.NewRequest("GET", u, nil) + if err != nil { + return nil, nil, err + } + return doRulesetRequest(ctx, client, req) +} + +// updateRepoRuleset updates a repository ruleset, working around the +// reviewer.id string marshaling bug in go-github. +func updateRepoRuleset(ctx context.Context, client *github.Client, owner, repo string, rulesetID int64, ruleset github.RepositoryRuleset) (*github.RepositoryRuleset, *github.Response, error) { + u := fmt.Sprintf("repos/%v/%v/rulesets/%v", owner, repo, rulesetID) + req, err := client.NewRequest("PUT", u, ruleset) + if err != nil { + return nil, nil, err + } + return doRulesetRequest(ctx, client, req) +} + +// createOrgRuleset creates an organization ruleset, working around the +// reviewer.id string marshaling bug in go-github. +func createOrgRuleset(ctx context.Context, client *github.Client, org string, ruleset github.RepositoryRuleset) (*github.RepositoryRuleset, *github.Response, error) { + u := fmt.Sprintf("orgs/%v/rulesets", org) + req, err := client.NewRequest("POST", u, ruleset) + if err != nil { + return nil, nil, err + } + return doRulesetRequest(ctx, client, req) +} + +// getOrgRuleset gets an organization ruleset, working around the +// reviewer.id string marshaling bug in go-github. +func getOrgRuleset(ctx context.Context, client *github.Client, org string, rulesetID int64) (*github.RepositoryRuleset, *github.Response, error) { + u := fmt.Sprintf("orgs/%v/rulesets/%v", org, rulesetID) + req, err := client.NewRequest("GET", u, nil) + if err != nil { + return nil, nil, err + } + return doRulesetRequest(ctx, client, req) +} + +// updateOrgRuleset updates an organization ruleset, working around the +// reviewer.id string marshaling bug in go-github. +func updateOrgRuleset(ctx context.Context, client *github.Client, org string, rulesetID int64, ruleset github.RepositoryRuleset) (*github.RepositoryRuleset, *github.Response, error) { + u := fmt.Sprintf("orgs/%v/rulesets/%v", org, rulesetID) + req, err := client.NewRequest("PUT", u, ruleset) + if err != nil { + return nil, nil, err + } + return doRulesetRequest(ctx, client, req) +} + func bypassActorsDiffSuppressFunc(k, o, n string, d *schema.ResourceData) bool { // If the length has changed, no need to suppress if k == "bypass_actors.#" { diff --git a/github/util_rules_test.go b/github/util_rules_test.go index 5a8253cc95..8d8c363e63 100644 --- a/github/util_rules_test.go +++ b/github/util_rules_test.go @@ -1,6 +1,7 @@ package github import ( + "encoding/json" "testing" "github.com/google/go-github/v84/github" @@ -1299,3 +1300,126 @@ func TestFlattenRulesetRepositoryPropertyTargetParameters_NilPropertyValues(t *t } } } + +func TestFixReviewerStringIDs(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "converts quoted integer id to unquoted", + input: `{"id": "12345", "type": "Team"}`, + expected: `{"id":12345, "type": "Team"}`, + }, + { + name: "leaves unquoted integer id unchanged", + input: `{"id": 12345, "type": "Team"}`, + expected: `{"id": 12345, "type": "Team"}`, + }, + { + name: "converts id without spaces after colon", + input: `{"id":"99999","type":"Team"}`, + expected: `{"id":99999,"type":"Team"}`, + }, + { + name: "handles multiple reviewer ids", + input: `[{"id": "111"}, {"id": "222"}]`, + expected: `[{"id":111}, {"id":222}]`, + }, + { + name: "does not convert non-digit string ids", + input: `{"id": "abc123", "type": "Team"}`, + expected: `{"id": "abc123", "type": "Team"}`, + }, + { + name: "does not convert empty string ids", + input: `{"id": "", "type": "Team"}`, + expected: `{"id": "", "type": "Team"}`, + }, + { + name: "handles full ruleset response with nested reviewer", + input: `{"id":1,"name":"test","rules":[{"type":"pull_request","parameters":{"required_reviewers":[{"reviewer":{"id":"12345","type":"Team"},"minimum_approvals":1}]}}]}`, + expected: `{"id":1,"name":"test","rules":[{"type":"pull_request","parameters":{"required_reviewers":[{"reviewer":{"id":12345,"type":"Team"},"minimum_approvals":1}]}}]}`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := fixReviewerStringIDs([]byte(tc.input)) + if string(result) != tc.expected { + t.Errorf("fixReviewerStringIDs(%q) = %q, want %q", tc.input, string(result), tc.expected) + } + }) + } +} + +func TestFixReviewerStringIDs_UnmarshalRoundTrip(t *testing.T) { + // Simulate the actual GitHub API response that causes the bug: + // reviewer.id is returned as a JSON string instead of a number. + apiResponse := `{ + "id": 999, + "name": "test-ruleset", + "target": "branch", + "source_type": "Repository", + "source": "test-repo", + "enforcement": "active", + "rules": [ + { + "type": "pull_request", + "parameters": { + "dismiss_stale_reviews_on_push": false, + "require_code_owner_review": false, + "require_last_push_approval": false, + "required_approving_review_count": 1, + "required_review_thread_resolution": false, + "required_reviewers": [ + { + "reviewer": { + "id": "12345", + "type": "Team" + }, + "file_patterns": ["*.go"], + "minimum_approvals": 1 + } + ] + } + } + ] + }` + + // Without the fix, this would fail with: + // json: cannot unmarshal string into Go struct field + // RulesetReviewer.id of type int64 + fixed := fixReviewerStringIDs([]byte(apiResponse)) + + var rs github.RepositoryRuleset + if err := json.Unmarshal(fixed, &rs); err != nil { + t.Fatalf("Failed to unmarshal fixed response: %v", err) + } + + if rs.GetID() != 999 { + t.Errorf("Expected ruleset ID 999, got %d", rs.GetID()) + } + + if rs.Rules == nil || rs.Rules.PullRequest == nil { + t.Fatal("Expected pull_request rule to be parsed") + } + + reviewers := rs.Rules.PullRequest.RequiredReviewers + if len(reviewers) != 1 { + t.Fatalf("Expected 1 required reviewer, got %d", len(reviewers)) + } + + if reviewers[0].Reviewer == nil { + t.Fatal("Expected reviewer to be non-nil") + } + + if reviewers[0].Reviewer.GetID() != 12345 { + t.Errorf("Expected reviewer ID 12345, got %d", reviewers[0].Reviewer.GetID()) + } + + if reviewers[0].Reviewer.GetType() == nil || *reviewers[0].Reviewer.GetType() != github.RulesetReviewerTypeTeam { + t.Errorf("Expected reviewer type Team, got %v", reviewers[0].Reviewer.GetType()) + } +}