refactor(providers): rename googleauth to googlecommon and dedup Vertex URL helpers#328
Conversation
…ex URL helpers
The googleauth package had outgrown its name. After the recent quota-project
work it already held HTTP transport concerns (X-Goog-User-Project injection)
that aren't strictly about auth. And there was an existing TODO at
vertex/vertex.go:350 noting that Vertex's URL transformation helpers were
copy-pasted into both vertex/vertex.go and gemini/gemini.go — three files'
worth of byte-identical code waiting for a shared home.
This PR:
- Renames internal/providers/googleauth → internal/providers/googlecommon,
files googleauth.go → auth.go to match. Updated import paths and package
qualifiers in vertex/ and gemini/ (the only callers).
- Extracts the three Vertex URL helpers (VertexBaseURLs,
VertexNativeBaseURLFromOpenAICompatibleBaseURL,
VertexOpenAICompatibleBaseURLFromNativeBaseURL) into
googlecommon/urls.go, exported. The signatures take plain strings rather
than the provider config struct so the helpers stay pure and avoid a
cross-package type dependency. Callers in vertex.go and gemini.go now call
googlecommon.VertexBaseURLs(cfg.BaseURL, cfg.VertexProject, cfg.VertexLocation).
- Removes the duplicated function definitions from vertex/vertex.go and
gemini/gemini.go, plus the TODO that flagged the duplication.
- Drops the dead `TokenSource` public function from auth.go. An audit showed
zero external callers — both providers migrated to FindCredentials after
the quota-project change.
Audit of remaining googlecommon exports (every one is shared by both
providers, confirming the package name fits):
Config vertex + gemini
Credentials vertex + gemini (via field access)
NormalizeAuthType vertex + gemini
HasServiceAccount vertex + gemini
FindCredentials vertex + gemini
HTTPClient vertex + gemini
Out of scope: hasResolvedProviderValue is still triple-defined (providers/config.go,
vertex/vertex.go, gemini/gemini.go). It's a generic ${VAR}-placeholder check
that doesn't fit a GCP-specific package; deferring to a follow-up.
Test plan: full `go test ./...` clean, including the three new VertexBaseURLs
tests already present in vertex_test.go. Pre-commit hooks (test-race, lint,
mod-tidy) pass. Net diff: -90 lines (29 added, 119 removed).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR consolidates Google authentication and Vertex AI URL utilities from separate local implementations into a shared ChangesGoogle Auth and Vertex URL Consolidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR renames
Confidence Score: 5/5Mechanical rename and deduplication with no logic changes; safe to merge. Every changed line is either an import path swap (googleauth to googlecommon), a removal of a byte-identical duplicate, or the deletion of a confirmed-dead function. The extracted URL helpers carry the same logic as both originals; the existing TestVertexBaseURLs table still covers all five URL derivation cases. No behaviour is altered. No files require special attention. googlecommon/urls.go holds the only net-new code and it is a straight copy of the removed originals. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Before
GA[googleauth/googleauth.go\nConfig, Credentials, NormalizeAuthType\nHasServiceAccount, FindCredentials\nHTTPClient, TokenSource dead]
VV1[vertex/vertex.go\n+ vertexBaseURLs\n+ vertexNativeBaseURLFrom...\n+ vertexOpenAICompatibleFrom...]
GG1[gemini/gemini.go\n+ vertexBaseURLs copy\n+ vertexNativeBaseURLFrom... copy\n+ vertexOpenAICompatibleFrom... copy]
VV1 -->|imports| GA
GG1 -->|imports| GA
end
subgraph After
AUTH[googlecommon/auth.go\nConfig, Credentials, NormalizeAuthType\nHasServiceAccount, FindCredentials\nHTTPClient]
URLS[googlecommon/urls.go\nVertexBaseURLs\nVertexNativeBaseURLFrom...\nVertexOpenAICompatibleFrom...]
VV2[vertex/vertex.go\nno local URL helpers]
GG2[gemini/gemini.go\nno local URL helpers]
VV2 -->|imports| AUTH
VV2 -->|imports| URLS
GG2 -->|imports| AUTH
GG2 -->|imports| URLS
end
Reviews (1): Last reviewed commit: "refactor(providers): rename googleauth t..." | Re-trigger Greptile |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary
The
googleauthpackage had outgrown its name. After the recent quota-project work (#325) it already held HTTP transport concerns (X-Goog-User-Projectinjection) that aren't strictly about auth. And the existing TODO atvertex/vertex.go:350noted that Vertex's URL transformation helpers were copy-pasted into bothvertex/vertex.goandgemini/gemini.go— byte-identical code waiting for a shared home (MD5-verified before changes).This PR:
internal/providers/googleauth→internal/providers/googlecommon, filesgoogleauth.go→auth.goto match.VertexBaseURLs,VertexNativeBaseURLFromOpenAICompatibleBaseURL,VertexOpenAICompatibleBaseURLFromNativeBaseURL) intogooglecommon/urls.go, exported. The signatures take plain strings rather than the provider config struct so the helpers stay pure and avoid a cross-package type dependency.vertex/vertex.goandgemini/gemini.go, plus the TODO that flagged the duplication.TokenSourcepublic function fromauth.go. An audit showed zero external callers — both providers migrated toFindCredentialsafter the quota-project change.Audit before renaming
Confirmed every truly-shared symbol in the old
googleauthis used by both providers, so the rename togooglecommonis justified:ConfigCredentials(via field access)NormalizeAuthTypeHasServiceAccountFindCredentialsHTTPClientTokenSourceOut of scope (follow-up candidate)
hasResolvedProviderValueis still defined three times (providers/config.go,vertex/vertex.go,gemini/gemini.go). It's a generic${VAR}-placeholder check that doesn't fit a GCP-specific package — deferring to a follow-up that picks a generic config-helper home for it.Risk
gomodel/internal/providers/googleauthmust update; since this is aninternal/package, that should be rare.VertexBaseURLssignature changed from(providers.ProviderConfig)to(baseURL, project, location string). Pure refactor; the only callers (in this repo) are updated in the same PR.TokenSourceremoved. The function had zero external callers — verified via grep across the whole repo before deletion. The package's own test that exercised the auth path was rewritten to useFindCredentials(which has always been the modern entry point).Test plan
make test-race,make lint,go mod tidy— all green via pre-commit hooks.go test ./...clean across the module.TestVertexBaseURLstest (now calling the exportedgooglecommon.VertexBaseURLs) still passes with all 5 table cases.Net diff: -90 lines (29 inserted, 119 removed) across 7 files.
🤖 Generated with Claude Code
Summary by CodeRabbit