Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ai/spec/what/sandbox-execution.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,5 @@ Behavioral specification for how workflow steps run inside ephemeral **sandboxes

- [PLANNED: OLS-2957] **Sandbox template management** UX and CRD ergonomics (base/derived lifecycle, versioning) may change operator/template coupling described in rules 2–4.
- [PLANNED: OLS-3038] **TLS verification and network policy** for agent traffic may replace permissive internal TLS client behavior.
- [PLANNED: OLS-3044] **Provider parity**: environment variable contract for non-Claude providers in templates MUST track sandbox image capabilities.
- **Sandbox provider selection**: Derived templates MUST set `LIGHTSPEED_AGENT_PROVIDER` from `LLMProvider.spec.type` (`Anthropic`/`AWSBedrock` → `claude`; `OpenAI`/`AzureOpenAI` → `openai`; `GoogleCloudVertex` → routed by `googleCloudVertex.modelProvider`: `Anthropic` → `claude`, `Google` → `gemini`, `OpenAI` → `openai`). Model env var follows the provider: `ANTHROPIC_MODEL`, `GEMINI_MODEL`, or `OPENAI_MODEL`.
- [PLANNED: OLS-2894] Support **multiple concurrent skills images** in template derivation beyond the first `skills` entry if product requires composite skill bundles.
22 changes: 22 additions & 0 deletions api/v1alpha1/llmprovider_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,20 @@ type AnthropicConfig struct {
URL string `json:"url,omitempty"`
}

// GoogleCloudVertexModelProvider selects which model provider stack talks to Vertex AI.
//
// +kubebuilder:validation:Enum=Anthropic;Google;OpenAI
type GoogleCloudVertexModelProvider string

const (
// GoogleCloudVertexModelProviderAnthropic uses Anthropic models on Vertex (Claude SDK).
GoogleCloudVertexModelProviderAnthropic GoogleCloudVertexModelProvider = "Anthropic"
// GoogleCloudVertexModelProviderGoogle uses Google models on Vertex (GenAI SDK).
GoogleCloudVertexModelProviderGoogle GoogleCloudVertexModelProvider = "Google"
// GoogleCloudVertexModelProviderOpenAI uses OpenAI-compatible models on Vertex (OpenAI SDK).
GoogleCloudVertexModelProviderOpenAI GoogleCloudVertexModelProvider = "OpenAI"
)

// GoogleCloudVertexConfig contains configuration for the Google Cloud Vertex AI provider.
type GoogleCloudVertexConfig struct {
// credentialsSecret references a Secret in the operator namespace
Expand Down Expand Up @@ -100,6 +114,13 @@ type GoogleCloudVertexConfig struct {
// +kubebuilder:validation:XValidation:rule="self.matches('^[a-z][a-z0-9-]*[a-z0-9]$')",message="region must contain only lowercase letters, digits, and hyphens, start with a letter, and not end with a hyphen"
Region string `json:"region,omitempty"`

// modelProvider selects which model provider stack talks to Vertex AI (required).
// "Anthropic" uses Anthropic models on Vertex; "Google" uses Google models on Vertex;
// "OpenAI" uses OpenAI-compatible models on Vertex.
// Enum is defined on GoogleCloudVertexModelProvider.
// +required
ModelProvider GoogleCloudVertexModelProvider `json:"modelProvider"`

// url is an optional override for the Vertex AI API endpoint.
// Only needed for custom deployments or API proxies.
// Must be a valid HTTP or HTTPS URL with a hostname. Paths and query
Expand Down Expand Up @@ -312,6 +333,7 @@ type LLMProviderSpec struct {
// name: llm-credentials
// projectID: my-gcp-project
// region: us-central1
// modelProvider: Anthropic
type LLMProvider struct {
metav1.TypeMeta `json:",inline"`

Expand Down
15 changes: 14 additions & 1 deletion config/crd/bases/agentic.openshift.io_llmproviders.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ spec:
AI provider (model specified on Agent, not here):\n\n\tapiVersion: agentic.openshift.io/v1alpha1\n\tkind:
LLMProvider\n\tmetadata:\n\t name: vertex-ai\n\tspec:\n\t type: GoogleCloudVertex\n\t
\ googleCloudVertex:\n\t credentialsSecret:\n\t name: llm-credentials\n\t
\ projectID: my-gcp-project\n\t region: us-central1"
\ projectID: my-gcp-project\n\t region: us-central1\n\t modelProvider:
Anthropic"
properties:
apiVersion:
description: |-
Expand Down Expand Up @@ -268,6 +269,17 @@ spec:
required:
- name
type: object
modelProvider:
description: |-
modelProvider selects which model provider stack talks to Vertex AI (required).
"Anthropic" uses Anthropic models on Vertex; "Google" uses Google models on Vertex;
"OpenAI" uses OpenAI-compatible models on Vertex.
Enum is defined on GoogleCloudVertexModelProvider.
enum:
- Anthropic
- Google
- OpenAI
type: string
projectID:
description: |-
projectID is the Google Cloud Project ID where Vertex AI is enabled.
Expand Down Expand Up @@ -316,6 +328,7 @@ spec:
rule: '!self.contains(''#'')'
required:
- credentialsSecret
- modelProvider
- projectID
- region
type: object
Expand Down
1 change: 1 addition & 0 deletions controller/proposal/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func testLLM(name string) *agenticv1alpha1.LLMProvider {
CredentialsSecret: agenticv1alpha1.SecretReference{Name: "llm-secret"},
ProjectID: "test-project",
Region: "us-central1",
ModelProvider: agenticv1alpha1.GoogleCloudVertexModelProviderAnthropic,
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion controller/proposal/sandbox_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

const (
defaultSandboxTimeout = 5 * time.Minute
defaultSandboxTimeout = 5 * time.Minute
defaultBaseTemplateName = "lightspeed-agent"
)

Expand Down
20 changes: 10 additions & 10 deletions controller/proposal/sandbox_agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ func newTestSandboxAgentCaller(sandbox *mockSandboxProvider, httpClient *mockHTT
fc := fake.NewClientBuilder().WithScheme(testScheme()).Build()
_ = fc.Create(context.Background(), fakeBaseTemplate())
return &SandboxAgentCaller{
Sandbox: sandbox,
K8sClient: fc,
ClientFactory: func(_ string) AgentHTTPClientInterface { return httpClient },
Namespace: "test-ns",
Timeout: 5 * time.Minute,
Sandbox: sandbox,
K8sClient: fc,
ClientFactory: func(_ string) AgentHTTPClientInterface { return httpClient },
Namespace: "test-ns",
Timeout: 5 * time.Minute,
}
}

Expand All @@ -73,11 +73,11 @@ func newTestSandboxAgentCallerWithProposal(sandbox *mockSandboxProvider, httpCli
Build()
_ = fc.Create(context.Background(), fakeBaseTemplate())
return &SandboxAgentCaller{
Sandbox: sandbox,
K8sClient: fc,
ClientFactory: func(_ string) AgentHTTPClientInterface { return httpClient },
Namespace: "test-ns",
Timeout: 5 * time.Minute,
Sandbox: sandbox,
K8sClient: fc,
ClientFactory: func(_ string) AgentHTTPClientInterface { return httpClient },
Namespace: "test-ns",
Timeout: 5 * time.Minute,
}
}

Expand Down
73 changes: 68 additions & 5 deletions controller/proposal/sandbox_templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ var sandboxTemplateGVK = schema.GroupVersionKind{
}

const (
agentModeEnvVar = "LIGHTSPEED_MODE"
agentModeEnvVar = "LIGHTSPEED_MODE"
sandboxAgentProviderEnvVar = "LIGHTSPEED_AGENT_PROVIDER"

vertexCredsMountPath = "/var/secrets/google"
vertexCredsFileName = "credentials.json"
Expand Down Expand Up @@ -245,14 +246,74 @@ func providerURL(llm *agenticv1alpha1.LLMProvider) string {
}
}

func vertexSandboxAgentProvider(mp agenticv1alpha1.GoogleCloudVertexModelProvider) (string, error) {
switch mp {
case agenticv1alpha1.GoogleCloudVertexModelProviderAnthropic:
return "claude", nil
case agenticv1alpha1.GoogleCloudVertexModelProviderGoogle:
return "gemini", nil
case agenticv1alpha1.GoogleCloudVertexModelProviderOpenAI:
return "openai", nil
default:
return "", fmt.Errorf("unsupported googleCloudVertex.modelProvider %q", mp)
}
}

func sandboxAgentProviderValue(llm *agenticv1alpha1.LLMProvider) (string, error) {
switch llm.Spec.Type {
case agenticv1alpha1.LLMProviderAnthropic, agenticv1alpha1.LLMProviderAWSBedrock:
return "claude", nil
case agenticv1alpha1.LLMProviderGoogleCloudVertex:
return vertexSandboxAgentProvider(llm.Spec.GoogleCloudVertex.ModelProvider)
case agenticv1alpha1.LLMProviderOpenAI, agenticv1alpha1.LLMProviderAzureOpenAI:
return "openai", nil
default:
return "", fmt.Errorf("unsupported LLM provider type %q", llm.Spec.Type)
}
}

func vertexModelEnvVar(mp agenticv1alpha1.GoogleCloudVertexModelProvider) string {
switch mp {
case agenticv1alpha1.GoogleCloudVertexModelProviderAnthropic:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we 3 different variable names for different providers just to map it back to single value in sandbox router? https://github.com/openshift/lightspeed-agentic-sandbox/blob/578aa4b5931571bc2eac05580e9faff09630887c/src/lightspeed_agentic/routes/__init__.py#L32

The only reason I've found was setting the default model, which could be simplified bu just setting the default based on the provider. Exposing 3 different vars for the same thing here feels like a leaky abtraction. IMO it would be better for the sandbox just to get MODEL_PROVIDER and MODEL, and doing the rest inside sandbox codebase.

return "ANTHROPIC_MODEL"
case agenticv1alpha1.GoogleCloudVertexModelProviderGoogle:
return "GEMINI_MODEL"
case agenticv1alpha1.GoogleCloudVertexModelProviderOpenAI:
return "OPENAI_MODEL"
default:
return ""
}
}

func modelEnvVarName(llm *agenticv1alpha1.LLMProvider) string {
switch llm.Spec.Type {
case agenticv1alpha1.LLMProviderOpenAI, agenticv1alpha1.LLMProviderAzureOpenAI:
return "OPENAI_MODEL"
case agenticv1alpha1.LLMProviderGoogleCloudVertex:
return vertexModelEnvVar(llm.Spec.GoogleCloudVertex.ModelProvider)
default:
return "ANTHROPIC_MODEL"
}
}

func patchLLMCredentials(tmpl *unstructured.Unstructured, llm *agenticv1alpha1.LLMProvider, model string) error {
secretName := credentialsSecretName(llm)

if err := addEnvFromSecret(tmpl, secretName); err != nil {
return fmt.Errorf("add credentials envFrom: %w", err)
}
if err := setEnvVar(tmpl, "ANTHROPIC_MODEL", model); err != nil {
return fmt.Errorf("set ANTHROPIC_MODEL: %w", err)

providerValue, err := sandboxAgentProviderValue(llm)
if err != nil {
return err
}
if err := setEnvVar(tmpl, sandboxAgentProviderEnvVar, providerValue); err != nil {
return fmt.Errorf("set %s: %w", sandboxAgentProviderEnvVar, err)
}

modelEnv := modelEnvVarName(llm)
if err := setEnvVar(tmpl, modelEnv, model); err != nil {
return fmt.Errorf("set %s: %w", modelEnv, err)
}

if u := providerURL(llm); u != "" {
Expand All @@ -264,8 +325,10 @@ func patchLLMCredentials(tmpl *unstructured.Unstructured, llm *agenticv1alpha1.L
switch llm.Spec.Type {
case agenticv1alpha1.LLMProviderGoogleCloudVertex:
cfg := llm.Spec.GoogleCloudVertex
if err := setEnvVar(tmpl, "CLAUDE_CODE_USE_VERTEX", "1"); err != nil {
return fmt.Errorf("set CLAUDE_CODE_USE_VERTEX: %w", err)
if cfg.ModelProvider == agenticv1alpha1.GoogleCloudVertexModelProviderAnthropic {
if err := setEnvVar(tmpl, "CLAUDE_CODE_USE_VERTEX", "1"); err != nil {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the operator really need to know about CLAUDE_CODE_USE_VERTEX. This feels like sandbox-specifc thing. Doing it here creates high coupling beween the specific sandbox implementation and the operator (that IMO should not care how exactly the sandbox is setup)

return fmt.Errorf("set CLAUDE_CODE_USE_VERTEX: %w", err)
}
}
if err := setEnvVar(tmpl, "GCP_PROJECT", cfg.ProjectID); err != nil {
return fmt.Errorf("set GCP_PROJECT: %w", err)
Expand Down
Loading