fix(deps): bump go-github to v88 and migrate to options-pattern constructor - #6537
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Atlantis’ GitHub integration to github.com/google/go-github/v88 and migrates construction of *github.Client to the new options-pattern constructor (error-returning), including the v88 Apps API rename.
Changes:
- Bump
go-githubfrom v83 → v88 across main, e2e, testdrive, and fixtures/mocks. - Migrate all
github.NewClient(...)call sites togithub.NewClient(opts...) (*Client, error)withWithHTTPClientand URL options. - Update API usage impacted by v88 (e.g.,
FindRepositoryInstallation→GetRepositoryInstallation) and adjust tests to useBaseURL().
Reviewed changes
Copilot reviewed 20 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
testdrive/testdrive.go |
Switch to options-pattern github.NewClient and handle constructor error. |
testdrive/github.go |
Update go-github import to v88. |
server/events/vcs/github/testdata/fixtures.go |
Update go-github import to v88 for test fixtures. |
server/events/vcs/github/mocks/mock_github_pull_request_getter.go |
Update go-github import to v88 in generated mock. |
server/events/vcs/github/instrumented_client.go |
Update go-github import to v88. |
server/events/vcs/github/credentials.go |
Construct clients via options pattern; configure base URL via WithURLs; handle new error return. |
server/events/vcs/github/client.go |
Construct clients via options pattern; configure enterprise URLs; wrap constructor errors. |
server/events/vcs/github/client_internal_test.go |
Update assertions to use BaseURL() accessor. |
server/events/plan_command_runner_test.go |
Update go-github import to v88 in tests. |
server/events/mocks/mock_github_pull_getter.go |
Update go-github import to v88 in generated mock. |
server/events/mocks/mock_event_parsing.go |
Update go-github import to v88 in generated mock. |
server/events/event_parser.go |
Update go-github import to v88. |
server/events/event_parser_test.go |
Update go-github import to v88 in tests. |
server/events/command_runner.go |
Update go-github import to v88. |
server/events/command_runner_test.go |
Update go-github import to v88 in tests. |
server/events/apply_command_runner_test.go |
Update go-github import to v88 in tests. |
server/controllers/events/github_request_validator.go |
Update go-github import to v88. |
server/controllers/events/events_controller.go |
Update go-github import to v88. |
server/controllers/events/events_controller_test.go |
Update go-github import to v88 in tests. |
server/controllers/events/events_controller_e2e_test.go |
Update go-github import to v88 in e2e-style controller test. |
go.mod |
Bump github.com/google/go-github/v83 → v88. |
go.sum |
Update checksums for go-github v88. |
e2e/go.mod |
Bump github.com/google/go-github/v83 → v88 for e2e module. |
e2e/go.sum |
Update checksums for go-github v88 in e2e module. |
e2e/github.go |
Migrate client construction to options-pattern; update Apps API call to GetRepositoryInstallation. |
Files not reviewed (3)
- server/events/mocks/mock_event_parsing.go: Language not supported
- server/events/mocks/mock_github_pull_getter.go: Language not supported
- server/events/vcs/github/mocks/mock_github_pull_request_getter.go: Language not supported
Comment on lines
133
to
+137
| apiURL := resolveGithubAPIURL(hostname) | ||
| // TODO: Deprecated: Use NewClient(httpClient).WithEnterpriseURLs(baseURL, uploadURL) instead | ||
| client, err = github.NewEnterpriseClient(apiURL.String(), apiURL.String(), transportWithRateLimit) //nolint:staticcheck | ||
| client, err = github.NewClient( | ||
| github.WithHTTPClient(transportWithRateLimit), | ||
| github.WithEnterpriseURLs(apiURL.String(), apiURL.String()), | ||
| ) |
go-github v87 introduced a breaking change (PR google/go-github#4201) that replaced github.NewClient(httpClient) *Client with NewClient(opts ...ClientOptionsFunc) (*Client, error) and removed NewEnterpriseClient. v88 also renamed App installation Find* methods to Get*, and Client.BaseURL is now a method rather than a *url.URL field. The renovate PR (#6528) bumped the module to v88 without migrating the calling code, breaking the Docker image build with: server/events/vcs/github/client.go:127: assignment mismatch: 1 variable but github.NewClient returns 2 values server/events/vcs/github/client.go:132: undefined: github.NewEnterpriseClient Migrate all call sites: - server/events/vcs/github/client.go: use WithHTTPClient + WithEnterpriseURLs - server/events/vcs/github/credentials.go: use WithHTTPClient + WithURLs (preserves prior behavior of overriding only the base URL) - server/events/vcs/github/client_internal_test.go: BaseURL() method - server/events/vcs/github/testdata/fixtures.go: bump import to v88 - testdrive/testdrive.go: handle the new error return - e2e/github.go: handle new error return; rename FindRepositoryInstallation to GetRepositoryInstallation Signed-off-by: PePe Amengual <2208324+jamengual@users.noreply.github.com>
jamengual
force-pushed
the
fix/go-github-v88-api-migration
branch
from
May 31, 2026 03:23
e9b8ffe to
d6a3cca
Compare
golangci-lint was bumped to v2.12.2 in #6512, which surfaced 16 lint findings in code unrelated to this PR. Fix them here so this PR's Linting check goes green: - gosec G705 (XSS in plain-text response writers): annotate respond() helpers in events, jobs, and locks controllers with #nosec G705 -- the response body is plain text, not HTML, and is never interpreted by a browser. - gosec G703 (path traversal write): annotate fileAppend and fileLineReplace in git_cred_writer.go with #nosec G703 -- the path is always derived from a trusted caller-supplied $HOME, mirroring the existing G304 nolint on the matching reads. - gosec G101 (hardcoded credentials in URLs): exclude G101 for *_test.go in .golangci.yml since test fixtures legitimately use http basic-auth URLs. Per-call sites that were already flagged also carry inline annotations for clarity. - staticcheck QF1012: replace WriteString(fmt.Sprintf(...)) with fmt.Fprintf in cmd/help_fmt.go and server/logging/simple_logger.go. - modernize stringscut: replace strings.Index + slicing with strings.Cut in two places in apply_step_runner.go. - modernize slicesbackward: replace manual reverse loop with slices.Backward in valid.GlobalCfg.MatchingRepo. Signed-off-by: PePe Amengual <2208324+jamengual@users.noreply.github.com>
CodeQL flagged the respond() helpers in the events, jobs, and locks controllers for reflected XSS because the response body interpolates user-controlled values into an HTTP response that browsers sniff as HTML by default. Explicitly set Content-Type to text/plain; charset=utf-8 before writing the body so browsers treat the payload as plain text rather than HTML, neutralizing the XSS vector. The existing #nosec G705 annotation is updated to reference the new mitigation. Signed-off-by: PePe Amengual <2208324+jamengual@users.noreply.github.com>
Contributor
Author
Status after
|
| Check | Status (prior run) | Notes |
|---|---|---|
| Tests | ✅ pass | |
| Linting | ✅ pass | Fixed 16 pre-existing golangci-lint v2.12.2 findings surfaced after #6512 merged |
| e2e-github | ✅ pass | (hook quota cleared) |
| Build Image (alpine/debian) | ⏳ in progress at last check | original migration goal — was failing on #6528 |
| Goss image tests | ✅ pass (all 6 arch/distros) | |
| CodeQL | ❌ → fixing now | 2 high-severity XSS alerts on respond() helpers. Fixed in 196ab9d7 by setting Content-Type: text/plain; charset=utf-8 |
| e2e-gitlab | ❌ infra | 401 invalid_token — GitLab API token expired; this also fails on main (see https://github.com/runatlantis/atlantis/actions?query=branch%3Amain+is%3Afailure). Not introduced by this PR |
| DCO | ✅ pass |
Out-of-scope fixes bundled in this PR
The lint and CodeQL findings were pre-existing on main and surfaced once golangci-lint was bumped to v2.12.2 in #6512. They were blocking the v88 migration from going green, so they're fixed here:
- gosec G705 / CodeQL reflected XSS:
respond()helpers now servetext/plain - gosec G703 path traversal:
git_cred_writer.goannotated (write paths derived from trusted$HOME) - gosec G101 hardcoded credentials in test URL fixtures:
.golangci.ymlexclusion for*_test.go - staticcheck QF1012:
fmt.FprintfreplacesWriteString(fmt.Sprintf(...))incmd/help_fmt.goandsimple_logger.go - modernize stringscut:
strings.Cutreplacesstrings.Index+ slicing inapply_step_runner.go - modernize slicesbackward:
slices.Backwardreplaces manual reverse loop invalid.GlobalCfg.MatchingRepo
Happy to split these into a follow-up PR if you'd prefer to keep this PR focused on the v88 migration alone.
Contributor
Author
|
All checks green except Final passing checks include:
|
lukemassa
approved these changes
Jun 3, 2026
lukemassa
left a comment
Contributor
There was a problem hiding this comment.
I'd personally prefer the unrelated linting issues be in a separate PR, but not enough to block the PR
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Bumps
github.com/google/go-githubfrom v83 → v88 and migrates all call sites to the new options-pattern client constructor introduced in v87 (google/go-github#4201). Supersedes #6528, which failed CI because it bumped the module without migrating the calling code.Breaking changes handled
github.NewClient(httpClient) *Clientgithub.NewClient(opts ...ClientOptionsFunc) (*Client, error)github.NewEnterpriseClient(base, upload, httpClient)github.NewClient(WithHTTPClient(c), WithEnterpriseURLs(base, upload))client.BaseURL(*url.URLfield)client.BaseURL()(method returningstring)Apps.FindRepositoryInstallationApps.GetRepositoryInstallation(v88, release notes)Files modified
server/events/vcs/github/client.go— useWithHTTPClient+WithEnterpriseURLs; handle new error returnserver/events/vcs/github/credentials.go— useWithHTTPClient+WithURLs(preserves prior behavior of overriding only the base URL while leaving the upload URL at its default)server/events/vcs/github/client_internal_test.go— callBaseURL()methodserver/events/vcs/github/testdata/fixtures.go— bump import to v88 (renovate missed this)testdrive/testdrive.go— handle new error returne2e/github.go— handle new error returns; renameFindRepositoryInstallation→GetRepositoryInstallationgo.mod/go.sum— drop v83, add v88Context
CI failure on #6528: https://github.com/runatlantis/atlantis/actions/runs/26621229042/job/78447430651
Upstream breaking-change PR: google/go-github#4201
Upstream release notes: v87, v88
Test plan
go build ./...passes locallygo vet ./...passes locallygo test -race ./server/events/vcs/github/...passes locally