Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions github/resource_github_organization_ruleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions github/resource_github_repository_ruleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down
109 changes: 109 additions & 0 deletions github/util_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ package github

import (
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"reflect"
"regexp"
"sort"

"github.com/google/go-github/v84/github"
Expand Down Expand Up @@ -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.#" {
Expand Down
124 changes: 124 additions & 0 deletions github/util_rules_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package github

import (
"encoding/json"
"testing"

"github.com/google/go-github/v84/github"
Expand Down Expand Up @@ -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())
}
}