fix(vertex): honor custom base_url as gateway prefix#4200
fix(vertex): honor custom base_url as gateway prefix#4200yyhhyyyyyy wants to merge 2 commits intoQuantumNous:mainfrom
Conversation
WalkthroughCentralized Vertex API URL construction into new helpers and updated adaptor code to delegate URL building; added tests for custom base URLs and validation; removed inline region/project/model parsing in task fetch logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TaskAdaptor
participant URLBuilder
participant VertexAPI
Client->>TaskAdaptor: submit/fetch request (includes upstream name, baseURL, credentials)
TaskAdaptor->>URLBuilder: BuildGoogleModelURL / BuildAnthropicModelURL / buildFetchOperationURL
URLBuilder-->>TaskAdaptor: returns constructed endpoint URL
TaskAdaptor->>VertexAPI: HTTP request to constructed URL (with query/key as needed)
VertexAPI-->>TaskAdaptor: operation / response
TaskAdaptor-->>Client: return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/channel/task/vertex/adaptor.go`:
- Around line 233-241: The code currently allows a missing project when baseURL
is provided, causing malformed endpoints; update the logic in the adaptor where
extractProjectFromOperationName(upstreamName) is used to require a non-empty
project regardless of baseURL: validate project (from
extractProjectFromOperationName) with strings.TrimSpace and return an error like
"cannot extract project from operation name" if empty, before calling
vertexcore.BuildGoogleModelURL (which uses vertexcore.DefaultAPIVersion,
project, region, modelName, "fetchPredictOperation"); keep the existing
modelName check and ensure both modelName and project are validated and fail
fast rather than relying on BuildAPIBaseURL behavior.
In `@relay/channel/vertex/url_builder.go`:
- Around line 28-32: The code currently appends "/"+version to any normalized
base URL; change this in the normalizeVertexBaseURL usage so you only append
when the normalized base does not already include the version: trim spaces and
slashes from both normalized and version, normalize trailing slashes on
normalized, then check if normalized already has the version as the final path
segment (e.g., strings.HasSuffix(normalized, "/"+version) or normalized ==
version) and only do normalized = normalized + "/" + version when that check is
false; update the logic around normalizeVertexBaseURL, baseURL, and version to
perform these defensive checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 64bc7259-1c4b-4253-926b-6a759a848ff7
📒 Files selected for processing (5)
relay/channel/task/vertex/adaptor.gorelay/channel/task/vertex/adaptor_test.gorelay/channel/vertex/adaptor.gorelay/channel/vertex/url_builder.gorelay/channel/vertex/url_builder_test.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
relay/channel/task/vertex/adaptor.go (1)
85-99:⚠️ Potential issue | 🟡 MinorFail fast when credentials are missing
project_idbefore building submit URL.At Line 98, URL construction proceeds even if
adc.ProjectIDis empty. With a custom gateway base URL, that can generate an invalid path and surface as a confusing upstream error instead of a clear local validation error.Suggested patch
func (a *TaskAdaptor) BuildRequestURL(info *relaycommon.RelayInfo) (string, error) { adc := &vertexcore.Credentials{} if err := common.Unmarshal([]byte(a.apiKey), adc); err != nil { return "", fmt.Errorf("failed to decode credentials: %w", err) } + if strings.TrimSpace(adc.ProjectID) == "" { + return "", fmt.Errorf("missing project_id in credentials") + } modelName := info.UpstreamModelName if modelName == "" { modelName = "veo-3.0-generate-001" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/task/vertex/adaptor.go` around lines 85 - 99, The code calls vertexcore.BuildGoogleModelURL using adc.ProjectID even when adc.ProjectID may be empty, which can produce invalid URLs; update the adaptor code (the block that decodes credentials into adc and then calls BuildGoogleModelURL) to validate adc.ProjectID after unmarshalling and return a clear error (e.g., "missing project_id in credentials") if it's empty before calling vertexcore.BuildGoogleModelURL or proceeding with URL construction, so callers get a fast, descriptive failure instead of an upstream error.
🧹 Nitpick comments (1)
relay/channel/vertex/url_builder_test.go (1)
83-194: Consider consolidating adaptor URL tests into a table-driven structure.The setup/assert pattern is repeated across multiple tests, which makes expansion noisier than needed.
♻️ Refactor sketch
+func assertRequestURL(t *testing.T, info *relaycommon.RelayInfo, want string) { + t.Helper() + adaptor := &Adaptor{} + adaptor.Init(info) + got, err := adaptor.GetRequestURL(info) + if err != nil { + t.Fatalf("GetRequestURL returned error: %v", err) + } + if got != want { + t.Fatalf("GetRequestURL() = %q, want %q", got, want) + } +} + -func TestAdaptorGetRequestURLUsesCustomBaseForGemini(t *testing.T) { - t.Parallel() - ... -} +func TestAdaptorGetRequestURLCases(t *testing.T) { + t.Parallel() + tests := []struct { + name string + info *relaycommon.RelayInfo + want string + }{ + // gemini/claude/open-source/api-key cases... + } + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assertRequestURL(t, tc.info, tc.want) + }) + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/vertex/url_builder_test.go` around lines 83 - 194, Consolidate the four near-duplicate tests (TestAdaptorGetRequestURLUsesCustomBaseForGemini, TestAdaptorGetRequestURLUsesCustomBaseForClaude, TestAdaptorGetRequestURLUsesCustomBaseForOpenSource, TestAdaptorGetRequestURLPreservesAPIKeyQueryWithCustomBase) into a single table-driven test that iterates test cases describing RelayInfo/ChannelMeta inputs and expected URLs; for each case create an Adaptor, call adaptor.Init(info) and adaptor.GetRequestURL(info), assert error nil and compare got vs want. Reuse mustMarshalCredentials where needed and include fields IsStream, OriginModelName, ChannelMeta.ApiVersion, ChannelBaseUrl, ApiKey and ChannelOtherSettings.VertexKeyType in the table rows so each scenario (gemini streaming, claude, open-source, API-key query) is covered. Ensure each case t.Run with a descriptive name and keep t.Parallel() per subtest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@relay/channel/task/vertex/adaptor.go`:
- Around line 85-99: The code calls vertexcore.BuildGoogleModelURL using
adc.ProjectID even when adc.ProjectID may be empty, which can produce invalid
URLs; update the adaptor code (the block that decodes credentials into adc and
then calls BuildGoogleModelURL) to validate adc.ProjectID after unmarshalling
and return a clear error (e.g., "missing project_id in credentials") if it's
empty before calling vertexcore.BuildGoogleModelURL or proceeding with URL
construction, so callers get a fast, descriptive failure instead of an upstream
error.
---
Nitpick comments:
In `@relay/channel/vertex/url_builder_test.go`:
- Around line 83-194: Consolidate the four near-duplicate tests
(TestAdaptorGetRequestURLUsesCustomBaseForGemini,
TestAdaptorGetRequestURLUsesCustomBaseForClaude,
TestAdaptorGetRequestURLUsesCustomBaseForOpenSource,
TestAdaptorGetRequestURLPreservesAPIKeyQueryWithCustomBase) into a single
table-driven test that iterates test cases describing RelayInfo/ChannelMeta
inputs and expected URLs; for each case create an Adaptor, call
adaptor.Init(info) and adaptor.GetRequestURL(info), assert error nil and compare
got vs want. Reuse mustMarshalCredentials where needed and include fields
IsStream, OriginModelName, ChannelMeta.ApiVersion, ChannelBaseUrl, ApiKey and
ChannelOtherSettings.VertexKeyType in the table rows so each scenario (gemini
streaming, claude, open-source, API-key query) is covered. Ensure each case
t.Run with a descriptive name and keep t.Parallel() per subtest.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4df6d6d2-ea15-4cc6-8ed4-40b9fb47fbad
📒 Files selected for processing (4)
relay/channel/task/vertex/adaptor.gorelay/channel/task/vertex/adaptor_test.gorelay/channel/vertex/url_builder.gorelay/channel/vertex/url_builder_test.go
✅ Files skipped from review due to trivial changes (1)
- relay/channel/task/vertex/adaptor_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- relay/channel/vertex/url_builder.go
Important
📝 变更描述 / Description
当前 Vertex 渠道即使配置了自定义 base_url,请求仍会走硬编码的官方上游地址,导致如 Cloudflare AI Gateway 这类自定义网关无法生效。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告,以证明变更生效)
2.修复后:已成功经过 Cloudflare AI Gateway
Summary by CodeRabbit
New Features
Bug Fixes
Tests