fix(vertex): add API key auth support to Embedding method - #4200
Conversation
The Embedding method unconditionally calls getAuthTokenSource(key), which attempts google.FindDefaultCredentials(). This fails in environments where GCP auth is provided externally via context headers (e.g. Workload Identity Federation) rather than Application Default Credentials. Other methods (ChatCompletion, Responses, ResponsesStream) already check key.Value and use it as an API key query parameter when set, allowing external auth via SetExtraHeaders to take effect. This commit applies the same pattern to Embedding for consistency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Confidence Score: 4/5The change is a one-method, pattern-matched fix consistent with every other auth branch in the same file; the OAuth fallback is unchanged and the API-key path is straightforward. The logic is correct and well-precedented within the file. The only gap is the complete absence of unit tests for Embedding() — neither the new API-key branch nor the existing OAuth path has any test coverage, so a future regression here would not be caught automatically. core/providers/vertex/vertex.go — specifically the new authQuery block in Embedding() and the missing test coverage for both auth branches Important Files Changed
Reviews (1): Last reviewed commit: "fix(vertex): add API key auth support to..." | Re-trigger Greptile |
| authQuery := "" | ||
| if key.Value.GetValue() != "" { | ||
| authQuery = fmt.Sprintf("key=%s", url.QueryEscape(key.Value.GetValue())) | ||
| } | ||
| completeURL := getCompleteURLForGeminiEndpoint(request.Model, region, projectID, projectNumber, ":predict") |
There was a problem hiding this comment.
Missing test coverage for the new auth branch
vertex_test.go has no Embedding test cases at all, so neither the new API-key path nor the fallback OAuth path is exercised by the test suite. The PR author's checklist also notes tests were not added. If a regression is introduced in the authQuery != "" branch (e.g., a bad URL construction) it will only surface at runtime. Consider adding at minimum a table-driven unit test that covers (1) key.Value set → ?key=… appended, OAuth skipped, and (2) key.Value empty → OAuth path invoked, no query param appended.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated ChangesVertex Embedding API Key Authentication
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
The Embedding method unconditionally calls getAuthTokenSource(key), which attempts google.FindDefaultCredentials(). This fails in environments where GCP auth is provided externally via context headers (e.g. Workload Identity Federation) rather than Application Default Credentials. Other methods (ChatCompletion, Responses, ResponsesStream) already check key.Value and use it as an API key query parameter when set, allowing external auth via SetExtraHeaders to take effect. This commit applies the same pattern to Embedding for consistency. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The Embedding method unconditionally calls getAuthTokenSource(key), which attempts google.FindDefaultCredentials(). This fails in environments where GCP auth is provided externally via context headers (e.g. Workload Identity Federation) rather than Application Default Credentials. Other methods (ChatCompletion, Responses, ResponsesStream) already check key.Value and use it as an API key query parameter when set, allowing external auth via SetExtraHeaders to take effect. This commit applies the same pattern to Embedding for consistency. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The Embedding method unconditionally calls getAuthTokenSource(key), which attempts google.FindDefaultCredentials(). This fails in environments where GCP auth is provided externally via context headers (e.g. Workload Identity Federation) rather than Application Default Credentials. Other methods (ChatCompletion, Responses, ResponsesStream) already check key.Value and use it as an API key query parameter when set, allowing external auth via SetExtraHeaders to take effect. This commit applies the same pattern to Embedding for consistency. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
The Vertex provider's
Embedding()method unconditionally callsgetAuthTokenSource(key), which attemptsgoogle.FindDefaultCredentials(). This fails in environments where GCP auth is provided externally via context headers (e.g. Workload Identity Federation) rather than Application Default Credentials.Other methods (
ChatCompletion,Responses,ResponsesStream) already handle this correctly by checkingkey.Valuefirst and skipping the internal auth when set — allowing auth provided viaBifrostContextKeyExtraHeadersto take effect. This PR applies the same pattern toEmbedding().Changes
authQuerycheck toEmbedding()incore/providers/vertex/vertex.go, matching the pattern used byChatCompletion(),Responses(), andResponsesStream()key.Valueis set, the value is appended as a?key=...query parameter andgetAuthTokenSource()is skippedkey.Valueis empty, the existing OAuth2 flow viagetAuthTokenSource()is preserved unchangedurlvariable tocompleteURLto avoid shadowing thenet/urlimport (needed forurl.QueryEscape)Type of change
Affected areas
How to test
go version go test ./...Additionally, to validate the fix end-to-end:
Valueand setAuthorizationheader viaBifrostContextKeyExtraHeaderskey.Valueand validAuthCredentialsstill work via the OAuth2 pathScreenshots/Recordings
Thats difficult to provide. We are using bifrost as the interal layer of a ai-gateway service we are building. With this change in play we can successfully route to vertex using our auth header in the same way we do for
Responsesetc.here is an example of a successful call:
Without this patch we always receive a key error instead:
Breaking changes
Related issues
N/A
Security considerations
No new auth mechanisms introduced. The change reuses the existing
authQuerypattern already present inChatCompletion(),Responses(), andResponsesStream(). The API key is URL-encoded viaurl.QueryEscapeconsistent with all other call sites.Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
New Features
Improvements