-
Notifications
You must be signed in to change notification settings - Fork 352
Add --gemini-api-target to AWF proxy for Gemini API routing #26060
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
acd068a
efbfdc2
e3d3877
8a208b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -315,6 +315,21 @@ func BuildAWFArgs(config AWFCommandConfig) []string { | |
| awfHelpersLog.Printf("Added --copilot-api-target=%s", copilotTarget) | ||
| } | ||
|
|
||
| // Add Gemini API target for the LLM gateway proxy. | ||
| // Unlike OpenAI/Anthropic/Copilot where AWF has built-in default routing, | ||
| // Gemini requires an explicit target so the proxy knows where to forward requests. | ||
| // Defaults to generativelanguage.googleapis.com when the engine is Gemini. | ||
| if geminiTarget := GetGeminiAPITarget(config.WorkflowData, config.EngineName); geminiTarget != "" { | ||
| awfArgs = append(awfArgs, "--gemini-api-target", geminiTarget) | ||
| awfHelpersLog.Printf("Added --gemini-api-target=%s", geminiTarget) | ||
| } | ||
|
|
||
| geminiBasePath := extractAPIBasePath(config.WorkflowData, "GEMINI_API_BASE_URL") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| if geminiBasePath != "" { | ||
| awfArgs = append(awfArgs, "--gemini-api-base-path", geminiBasePath) | ||
| awfHelpersLog.Printf("Added --gemini-api-base-path=%s", geminiBasePath) | ||
| } | ||
|
Comment on lines
+318
to
+331
|
||
|
|
||
| // Add SSL Bump support for HTTPS content inspection (v0.9.0+) | ||
| sslBumpArgs := getSSLBumpArgs(firewallConfig) | ||
| awfArgs = append(awfArgs, sslBumpArgs...) | ||
|
|
@@ -491,6 +506,7 @@ func extractAPIBasePath(workflowData *WorkflowData, envVar string) string { | |
| // - Codex: OPENAI_BASE_URL → --openai-api-target | ||
| // - Claude: ANTHROPIC_BASE_URL → --anthropic-api-target | ||
| // - Copilot: GITHUB_COPILOT_BASE_URL → --copilot-api-target (fallback when api-target not set) | ||
| // - Gemini: GEMINI_API_BASE_URL → --gemini-api-target (default: generativelanguage.googleapis.com) | ||
| // | ||
| // Returns empty string if neither source is configured. | ||
| func GetCopilotAPITarget(workflowData *WorkflowData) string { | ||
|
|
@@ -503,6 +519,33 @@ func GetCopilotAPITarget(workflowData *WorkflowData) string { | |
| return extractAPITargetHost(workflowData, "GITHUB_COPILOT_BASE_URL") | ||
| } | ||
|
|
||
| // DefaultGeminiAPITarget is the default Gemini API endpoint hostname. | ||
| // AWF's proxy sidecar needs this target to forward Gemini API requests, since | ||
| // unlike OpenAI/Anthropic/Copilot, the proxy has no built-in default handler for Gemini. | ||
| const DefaultGeminiAPITarget = "generativelanguage.googleapis.com" | ||
|
|
||
| // GetGeminiAPITarget returns the effective Gemini API target hostname for the LLM gateway proxy. | ||
| // Unlike other engines where AWF has built-in default routing, Gemini requires an explicit target. | ||
| // | ||
| // Resolution order: | ||
| // 1. GEMINI_API_BASE_URL in engine.env (custom endpoint) | ||
| // 2. Default: generativelanguage.googleapis.com (when engine is "gemini") | ||
| // | ||
| // Returns empty string if the engine is not Gemini and no custom GEMINI_API_BASE_URL is configured. | ||
| func GetGeminiAPITarget(workflowData *WorkflowData, engineName string) string { | ||
| // Check for custom GEMINI_API_BASE_URL in engine.env | ||
| if customTarget := extractAPITargetHost(workflowData, "GEMINI_API_BASE_URL"); customTarget != "" { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The string literal |
||
| return customTarget | ||
| } | ||
|
|
||
| // Default to the standard Gemini API endpoint when engine is Gemini | ||
| if engineName == "gemini" { | ||
| return DefaultGeminiAPITarget | ||
| } | ||
|
|
||
| return "" | ||
| } | ||
|
|
||
| // ComputeAWFExcludeEnvVarNames returns the list of environment variable names that must be | ||
| // excluded from the agent container's visible environment via AWF's --exclude-env flag. | ||
| // | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -919,3 +919,218 @@ func TestAWFSupportsCliProxy(t *testing.T) { | |
| }) | ||
| } | ||
| } | ||
|
|
||
| // TestGetGeminiAPITarget tests the GetGeminiAPITarget helper that resolves the effective | ||
| // Gemini API target from GEMINI_API_BASE_URL in engine.env or the default endpoint. | ||
| func TestGetGeminiAPITarget(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| workflowData *WorkflowData | ||
| engineName string | ||
| expected string | ||
| }{ | ||
| { | ||
| name: "returns default target for gemini engine with no custom URL", | ||
| workflowData: &WorkflowData{ | ||
| EngineConfig: &EngineConfig{ | ||
| ID: "gemini", | ||
| }, | ||
| }, | ||
| engineName: "gemini", | ||
| expected: "generativelanguage.googleapis.com", | ||
| }, | ||
| { | ||
| name: "custom GEMINI_API_BASE_URL takes precedence over default", | ||
| workflowData: &WorkflowData{ | ||
| EngineConfig: &EngineConfig{ | ||
| ID: "gemini", | ||
| Env: map[string]string{ | ||
| "GEMINI_API_BASE_URL": "https://gemini-proxy.internal.company.com/v1", | ||
| }, | ||
| }, | ||
| }, | ||
| engineName: "gemini", | ||
| expected: "gemini-proxy.internal.company.com", | ||
| }, | ||
| { | ||
| name: "returns empty for non-gemini engine without custom URL", | ||
| workflowData: &WorkflowData{ | ||
| EngineConfig: &EngineConfig{ | ||
| ID: "claude", | ||
| }, | ||
| }, | ||
| engineName: "claude", | ||
| expected: "", | ||
| }, | ||
| { | ||
| name: "returns empty when workflowData is nil", | ||
| workflowData: nil, | ||
| engineName: "gemini", | ||
| expected: "generativelanguage.googleapis.com", | ||
| }, | ||
|
Comment on lines
+966
to
+970
|
||
| { | ||
| name: "returns custom target for non-gemini engine with GEMINI_API_BASE_URL", | ||
| workflowData: &WorkflowData{ | ||
| EngineConfig: &EngineConfig{ | ||
| ID: "custom", | ||
| Env: map[string]string{ | ||
| "GEMINI_API_BASE_URL": "https://custom-proxy.example.com", | ||
| }, | ||
| }, | ||
| }, | ||
| engineName: "custom", | ||
| expected: "custom-proxy.example.com", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| result := GetGeminiAPITarget(tt.workflowData, tt.engineName) | ||
| assert.Equal(t, tt.expected, result, "GetGeminiAPITarget should return expected hostname") | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // TestAWFGeminiAPITargetFlags tests that BuildAWFArgs includes --gemini-api-target flag | ||
| // for the Gemini engine with default and custom endpoints. | ||
| func TestAWFGeminiAPITargetFlags(t *testing.T) { | ||
| t.Run("includes default gemini-api-target flag for gemini engine", func(t *testing.T) { | ||
| workflowData := &WorkflowData{ | ||
| Name: "test-workflow", | ||
| EngineConfig: &EngineConfig{ | ||
| ID: "gemini", | ||
| }, | ||
| NetworkPermissions: &NetworkPermissions{ | ||
| Firewall: &FirewallConfig{ | ||
| Enabled: true, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| config := AWFCommandConfig{ | ||
| EngineName: "gemini", | ||
| WorkflowData: workflowData, | ||
| AllowedDomains: "github.com", | ||
| } | ||
|
|
||
| args := BuildAWFArgs(config) | ||
| argsStr := strings.Join(args, " ") | ||
|
|
||
| assert.Contains(t, argsStr, "--gemini-api-target", "Should include --gemini-api-target flag") | ||
| assert.Contains(t, argsStr, "generativelanguage.googleapis.com", "Should include default Gemini API hostname") | ||
| }) | ||
|
|
||
| t.Run("includes custom gemini-api-target flag when GEMINI_API_BASE_URL is configured", func(t *testing.T) { | ||
| workflowData := &WorkflowData{ | ||
| Name: "test-workflow", | ||
| EngineConfig: &EngineConfig{ | ||
| ID: "gemini", | ||
| Env: map[string]string{ | ||
| "GEMINI_API_BASE_URL": "https://gemini-proxy.internal.company.com/v1", | ||
| "GEMINI_API_KEY": "${{ secrets.GEMINI_PROXY_KEY }}", | ||
| }, | ||
| }, | ||
| NetworkPermissions: &NetworkPermissions{ | ||
| Firewall: &FirewallConfig{ | ||
| Enabled: true, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| config := AWFCommandConfig{ | ||
| EngineName: "gemini", | ||
| WorkflowData: workflowData, | ||
| AllowedDomains: "github.com", | ||
| } | ||
|
|
||
| args := BuildAWFArgs(config) | ||
| argsStr := strings.Join(args, " ") | ||
|
|
||
| assert.Contains(t, argsStr, "--gemini-api-target", "Should include --gemini-api-target flag") | ||
| assert.Contains(t, argsStr, "gemini-proxy.internal.company.com", "Should include custom Gemini hostname") | ||
| }) | ||
|
|
||
| t.Run("does not include gemini-api-target for non-gemini engine without custom URL", func(t *testing.T) { | ||
| workflowData := &WorkflowData{ | ||
| Name: "test-workflow", | ||
| EngineConfig: &EngineConfig{ | ||
| ID: "claude", | ||
| }, | ||
| NetworkPermissions: &NetworkPermissions{ | ||
| Firewall: &FirewallConfig{ | ||
| Enabled: true, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| config := AWFCommandConfig{ | ||
| EngineName: "claude", | ||
| WorkflowData: workflowData, | ||
| AllowedDomains: "github.com", | ||
| } | ||
|
|
||
| args := BuildAWFArgs(config) | ||
| argsStr := strings.Join(args, " ") | ||
|
|
||
| assert.NotContains(t, argsStr, "--gemini-api-target", "Should not include --gemini-api-target for non-gemini engine") | ||
| }) | ||
|
|
||
| t.Run("includes gemini-api-base-path when custom URL has path component", func(t *testing.T) { | ||
| workflowData := &WorkflowData{ | ||
| Name: "test-workflow", | ||
| EngineConfig: &EngineConfig{ | ||
| ID: "gemini", | ||
| Env: map[string]string{ | ||
| "GEMINI_API_BASE_URL": "https://gemini-proxy.company.com/serving-endpoints", | ||
| "GEMINI_API_KEY": "${{ secrets.GEMINI_PROXY_KEY }}", | ||
| }, | ||
| }, | ||
| NetworkPermissions: &NetworkPermissions{ | ||
| Firewall: &FirewallConfig{ | ||
| Enabled: true, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| config := AWFCommandConfig{ | ||
| EngineName: "gemini", | ||
| WorkflowData: workflowData, | ||
| AllowedDomains: "github.com", | ||
| } | ||
|
|
||
| args := BuildAWFArgs(config) | ||
| argsStr := strings.Join(args, " ") | ||
|
|
||
| assert.Contains(t, argsStr, "--gemini-api-base-path", "Should include --gemini-api-base-path flag") | ||
| assert.Contains(t, argsStr, "/serving-endpoints", "Should include the path component") | ||
| }) | ||
| } | ||
|
|
||
| // TestGeminiEngineIncludesGeminiAPITarget tests that the Gemini engine execution | ||
| // step includes --gemini-api-target when firewall is enabled. | ||
| func TestGeminiEngineIncludesGeminiAPITarget(t *testing.T) { | ||
| workflowData := &WorkflowData{ | ||
| Name: "test-workflow", | ||
| EngineConfig: &EngineConfig{ | ||
| ID: "gemini", | ||
| }, | ||
| NetworkPermissions: &NetworkPermissions{ | ||
| Firewall: &FirewallConfig{ | ||
| Enabled: true, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| engine := NewGeminiEngine() | ||
| steps := engine.GetExecutionSteps(workflowData, "test.log") | ||
|
|
||
| if len(steps) < 2 { | ||
| t.Fatal("Expected at least two execution steps (settings + execution)") | ||
| } | ||
|
|
||
| // steps[0] = Write Gemini Settings, steps[1] = Execute Gemini CLI | ||
| stepContent := strings.Join(steps[1], "\n") | ||
|
|
||
| assert.Contains(t, stepContent, "--gemini-api-target", "Should include --gemini-api-target flag") | ||
| assert.Contains(t, stepContent, "generativelanguage.googleapis.com", "Should include default Gemini API hostname") | ||
| } | ||
|
Comment on lines
+994
to
+1136
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence says "Three environment variables" but then lists four (OPENAI_BASE_URL, ANTHROPIC_BASE_URL, GITHUB_COPILOT_BASE_URL, GEMINI_API_BASE_URL). Update the count or rephrase (e.g., "The following environment variables...") to avoid confusing readers.