fix: handle string-typed reviewer.id in ruleset API responses#3341
fix: handle string-typed reviewer.id in ruleset API responses#3341dpwspoon-hi wants to merge 1 commit intointegrations:mainfrom
Conversation
The GitHub API returns `required_reviewers.reviewer.id` as a JSON string (e.g., `"12345"`), but go-github's `RulesetReviewer.ID` field is `*int64`. This causes `json.Unmarshal` to fail, preventing the provider from recording the resource in state after creation — leading to orphaned rulesets on every retry. Work around this by replacing direct go-github API calls for ruleset CRUD with thin wrappers that use `client.BareDo` to obtain the raw response body, apply a regex fix to convert quoted integer IDs to unquoted integers, and then unmarshal into the go-github types. Fixes integrations#3340 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with |
deiga
left a comment
There was a problem hiding this comment.
Sorry, but we will not accept hacky workarounds for possible issues in go-github. Either find a reasonable way to fix it in the provider or submit a PR to go-github
I took a stab at this here in |
|
@mmelodyRTR Loving this! Thanks for diving into this |
Summary
Fixes #3340 and #3214
The GitHub API returns
required_reviewers.reviewer.idas a JSON string (e.g.,"12345"), but go-github'sRulesetReviewer.IDfield is*int64. Standard Go JSON unmarshaling rejects this with:The POST/PUT succeeds (the ruleset is created/updated on GitHub), but the provider cannot parse the response — so the resource is never recorded in Terraform state, creating orphaned rulesets on every retry.
Why existing tests didn't catch this
The acceptance test
TestAccGithubRepositoryRuleset_requiredReviewers(added in #3073) only runs when a maintainer adds theacctestlabel, and the CI matrix (anonymous,individual,organization) requires an org-mode run to pass theskipUnlessHasOrgsprecheck. The unit tests forexpandRequiredReviewers/flattenRequiredReviewersonly exercise in-memory Go struct round-trips — they never parse raw JSON from the API, so they never hit the string→int64 mismatch that lives inside go-github'sjson.Unmarshalpath.Approach
Rather than patching go-github upstream, this adds thin wrapper functions around the ruleset CRUD API calls in
util_rules.go. The wrappers useclient.BareDo()to obtain the raw HTTP response body, apply a targeted regex ("id"\s*:\s*"(\d+)"→"id":$1) to convert quoted integer IDs to unquoted integers, then unmarshal into the standard go-github types.This approach:
"id"field returned as a quoted integer in ruleset responses isreviewer.id(the top-level ruleset ID and other numeric fields are already returned as JSON numbers)Changes
github/util_rules.go: AddedfixReviewerStringIDs()regex helper,doRulesetRequest()for raw response handling, and 6 wrapper functions (createRepoRuleset,getRepoRuleset,updateRepoRuleset,createOrgRuleset,getOrgRuleset,updateOrgRuleset)github/resource_github_repository_ruleset.go: Replaced 4 direct go-github calls with wrapper functionsgithub/resource_github_organization_ruleset.go: Replaced 4 direct go-github calls with wrapper functionsgithub/util_rules_test.go: Added unit tests forfixReviewerStringIDs(7 cases) and an end-to-end unmarshal round-trip test that simulates the actual buggy API responseTest plan
fixReviewerStringIDscovering: quoted integer conversion, unquoted passthrough, no-space formatting, multiple IDs, non-digit strings, empty strings, full nested responseTestExpandRequiredReviewers,TestFlattenRequiredReviewers,TestRoundTripRequiredReviewersstill passgo vet ./...cleanTestAccGithubRepositoryRuleset_requiredReviewers— requiresacctestlabel + org-mode CI run to exercise the real API round-trip