diff --git a/go/api/adk/types.go b/go/api/adk/types.go index 92d1f9293..3c815644c 100644 --- a/go/api/adk/types.go +++ b/go/api/adk/types.go @@ -584,7 +584,10 @@ func (c *AgentCompressionConfig) UnmarshalJSON(data []byte) error { // See `python/packages/kagent-adk/src/kagent/adk/types.py` for the python version of this type AgentConfig struct { - Model Model `json:"model"` + // Model is omitted when unset rather than emitted as "model":null. The python + // runtime requires model to be an object when the key is present, so a null + // value is never valid input for it. + Model Model `json:"model,omitempty"` Description string `json:"description"` Instruction string `json:"instruction"` HttpTools []HttpMcpServerConfig `json:"http_tools,omitempty"` diff --git a/go/core/internal/controller/translator/agent/manifest_builder.go b/go/core/internal/controller/translator/agent/manifest_builder.go index bc153b96e..efe0f342e 100644 --- a/go/core/internal/controller/translator/agent/manifest_builder.go +++ b/go/core/internal/controller/translator/agent/manifest_builder.go @@ -202,7 +202,9 @@ func (a *adkApiTranslator) buildConfigSecret( var volumes []corev1.Volume var mounts []corev1.VolumeMount - if cfg != nil { + renderAgentConfig := needsAgentConfig(manifestCtx.agent, cfg) + + if renderAgentConfig { bCfg, err := json.Marshal(cfg) if err != nil { return nil, err @@ -230,7 +232,7 @@ func (a *adkApiTranslator) buildConfigSecret( srtSettingsJSON = string(bSRTSettings) } - if cfg != nil || srtSettingsJSON != "" { + if renderAgentConfig || srtSettingsJSON != "" { secretData := modelConfigSecretHashBytes if secretData == nil { secretData = []byte{} @@ -356,6 +358,18 @@ func buildPodRuntime( }, nil } +// needsAgentConfig reports whether the agent consumes the controller-rendered +// config.json. BYO agents run their own image and do not share the declarative +// runtime's configuration schema, so they must not be given the config volume: +// the config rendered for them carries no model, which the declarative runtime's +// schema requires. +func needsAgentConfig(agent v1alpha2.AgentObject, cfg *adk.AgentConfig) bool { + if cfg == nil { + return false + } + return agent.GetAgentSpec().Type != v1alpha2.AgentType_BYO +} + func needsSRTSettings(agent v1alpha2.AgentObject, sandboxCfg *v1alpha2.SandboxConfig) bool { spec := agent.GetAgentSpec() if spec.Type == v1alpha2.AgentType_BYO { diff --git a/go/core/internal/controller/translator/agent/manifest_builder_test.go b/go/core/internal/controller/translator/agent/manifest_builder_test.go index 8c01a2af5..3895cdcb3 100644 --- a/go/core/internal/controller/translator/agent/manifest_builder_test.go +++ b/go/core/internal/controller/translator/agent/manifest_builder_test.go @@ -1,9 +1,11 @@ package agent import ( + "context" "encoding/json" "testing" + "github.com/kagent-dev/kagent/go/api/adk" "github.com/kagent-dev/kagent/go/api/v1alpha2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -96,6 +98,127 @@ func TestNeedsSRTSettings(t *testing.T) { } } +func TestNeedsAgentConfig(t *testing.T) { + declarativeAgent := &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{Name: "decl", Namespace: "default"}, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{}, + }, + } + byoAgent := &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{Name: "byo", Namespace: "default"}, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_BYO, + BYO: &v1alpha2.BYOAgentSpec{}, + }, + } + cfg := &adk.AgentConfig{Description: "a test agent"} + + if needsAgentConfig(declarativeAgent, nil) { + t.Fatal("a nil config should never be rendered") + } + if !needsAgentConfig(declarativeAgent, cfg) { + t.Fatal("declarative agents should get the rendered agent config") + } + if needsAgentConfig(byoAgent, cfg) { + t.Fatal("BYO agents should not get the rendered agent config") + } +} + +func byoManifestContext() manifestContext { + return manifestContext{ + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{Name: "byo", Namespace: "default"}, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_BYO, + BYO: &v1alpha2.BYOAgentSpec{}, + }, + }, + deployment: &resolvedDeployment{}, + } +} + +// TestBuildConfigSecret_BYOOmitsAgentConfig guards against handing BYO agents the +// config rendered for the declarative runtime. That config carries no model, and the +// runtime schema requires one, so a BYO image that loads /config/config.json on +// startup fails validation and crashloops. +func TestBuildConfigSecret_BYOOmitsAgentConfig(t *testing.T) { + translator := &adkApiTranslator{} + // The minimal config the compiler builds for BYO agents: a description, no model. + cfg := &adk.AgentConfig{Description: "A BYO test agent"} + + got, err := translator.buildConfigSecret(context.Background(), byoManifestContext(), cfg, nil, nil, nil) + if err != nil { + t.Fatalf("buildConfigSecret() error = %v", err) + } + + if data := got.secret.StringData["config.json"]; data != "" { + t.Fatalf("config.json = %q, want empty for BYO agents", data) + } + if len(got.volumes) != 0 { + t.Fatalf("volumes = %#v, want none for BYO agents", got.volumes) + } + if len(got.mounts) != 0 { + t.Fatalf("mounts = %#v, want none for BYO agents", got.mounts) + } +} + +// TestBuildConfigSecret_BYOWithSandboxMountsOnlySRTSettings covers the one case where a +// BYO agent still needs the config volume. The agent config stays empty; only the srt +// settings are populated. +func TestBuildConfigSecret_BYOWithSandboxMountsOnlySRTSettings(t *testing.T) { + translator := &adkApiTranslator{} + cfg := &adk.AgentConfig{Description: "A BYO test agent"} + + got, err := translator.buildConfigSecret(context.Background(), byoManifestContext(), cfg, &v1alpha2.SandboxConfig{}, nil, nil) + if err != nil { + t.Fatalf("buildConfigSecret() error = %v", err) + } + + if data := got.secret.StringData["config.json"]; data != "" { + t.Fatalf("config.json = %q, want empty for BYO agents", data) + } + if got.secret.StringData["srt-settings.json"] == "" { + t.Fatal("srt-settings.json should be populated for sandboxed BYO agents") + } + if len(got.mounts) != 1 || got.mounts[0].MountPath != "/config" { + t.Fatalf("mounts = %#v, want a single /config mount", got.mounts) + } +} + +// TestBuildConfigSecret_DeclarativeKeepsAgentConfig pins the declarative path, which +// must keep receiving the rendered config and its volume. +func TestBuildConfigSecret_DeclarativeKeepsAgentConfig(t *testing.T) { + translator := &adkApiTranslator{} + manifestCtx := manifestContext{ + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{Name: "decl", Namespace: "default"}, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{}, + }, + }, + deployment: &resolvedDeployment{}, + } + cfg := &adk.AgentConfig{Description: "a declarative agent"} + + got, err := translator.buildConfigSecret(context.Background(), manifestCtx, cfg, nil, nil, nil) + if err != nil { + t.Fatalf("buildConfigSecret() error = %v", err) + } + + if got.secret.StringData["config.json"] == "" { + t.Fatal("config.json should be populated for declarative agents") + } + if len(got.volumes) != 1 || got.volumes[0].Name != "config" { + t.Fatalf("volumes = %#v, want a single config volume", got.volumes) + } + if len(got.mounts) != 1 || got.mounts[0].MountPath != "/config" { + t.Fatalf("mounts = %#v, want a single /config mount", got.mounts) + } +} + func TestBuildConfigSecretData_OmitsEmptySRTSettings(t *testing.T) { data := buildConfigSecretData(`{"app":"ok"}`, `{"card":"ok"}`, "") diff --git a/go/core/internal/controller/translator/agent/testdata/outputs/byo_agent.json b/go/core/internal/controller/translator/agent/testdata/outputs/byo_agent.json index e1f03e859..ecb6e930b 100644 --- a/go/core/internal/controller/translator/agent/testdata/outputs/byo_agent.json +++ b/go/core/internal/controller/translator/agent/testdata/outputs/byo_agent.json @@ -28,8 +28,7 @@ }, "config": { "description": "A BYO test agent", - "instruction": "", - "model": null + "instruction": "" }, "manifest": [ { @@ -58,7 +57,7 @@ }, "stringData": { "agent-card.json": "{\n \"defaultInputModes\": [\n \"text\"\n ],\n \"defaultOutputModes\": [\n \"text\"\n ],\n \"description\": \"A BYO test agent\",\n \"name\": \"byo_agent\",\n \"version\": \"\",\n \"skills\": [],\n \"capabilities\": {\n \"streaming\": true\n },\n \"supportedInterfaces\": [\n {\n \"url\": \"http://byo-agent.test:8080\",\n \"protocolBinding\": \"JSONRPC\",\n \"protocolVersion\": \"0.3\"\n },\n {\n \"url\": \"http://byo-agent.test:8080\",\n \"protocolBinding\": \"JSONRPC\",\n \"protocolVersion\": \"1.0\"\n }\n ],\n \"url\": \"http://byo-agent.test:8080\",\n \"protocolVersion\": \"0.3\",\n \"preferredTransport\": \"JSONRPC\"\n}", - "config.json": "{\"model\":null,\"description\":\"A BYO test agent\",\"instruction\":\"\"}" + "config.json": "" } }, { @@ -127,7 +126,7 @@ "template": { "metadata": { "annotations": { - "kagent.dev/config-hash": "14721439369706619567" + "kagent.dev/config-hash": "0" }, "labels": { "app": "kagent", @@ -187,10 +186,6 @@ } }, "volumeMounts": [ - { - "mountPath": "/config", - "name": "config" - }, { "mountPath": "/var/run/secrets/tokens", "name": "kagent-token" @@ -200,12 +195,6 @@ ], "serviceAccountName": "byo-agent", "volumes": [ - { - "name": "config", - "secret": { - "secretName": "byo-agent" - } - }, { "name": "kagent-token", "projected": {