From 3450fc606801450b091d763fd3a7e93c302d1669 Mon Sep 17 00:00:00 2001 From: Azeez Syed Date: Sat, 6 Dec 2025 22:39:28 +0530 Subject: [PATCH] feat(api): use nested structure for ModelConfig API key secret Add nested APIKeyConfig structure for ModelConfig CRD to provide consistent pattern with TLS configuration. This deprecates the flat apiKeySecret/apiKeySecretKey fields in favor of apiKey.secretRef and apiKey.secretKey. Changes: - Add APIKeyConfig struct with SecretRef and SecretKey fields - Add APIKey field to ModelConfigSpec - Mark apiKeySecret and apiKeySecretKey as deprecated - Add validation rules to prevent mixed usage - Add helper methods GetAPIKeySecretRef() and IsUsingDeprecatedAPIKeyFields() - Update reconciler to log deprecation warning - Update translator and controller to use helper method - Update HTTP handlers for backwards compatibility - Add unit and golden tests for nested API key support Closes #1130 Signed-off-by: Azeez Syed --- go/api/v1alpha2/modelconfig_types.go | 47 ++- go/api/v1alpha2/zz_generated.deepcopy.go | 20 ++ .../crd/bases/kagent.dev_modelconfigs.yaml | 35 ++- .../controller/modelconfig_controller.go | 4 +- .../controller/reconciler/reconciler.go | 15 +- .../translator/agent/adk_api_translator.go | 38 ++- .../agent_with_nested_apikey_modelconfig.yaml | 28 ++ .../agent_with_nested_apikey_modelconfig.json | 292 ++++++++++++++++++ .../httpserver/handlers/modelconfig.go | 30 +- .../httpserver/handlers/modelconfig_test.go | 52 ++++ go/pkg/client/api/types.go | 16 +- 11 files changed, 529 insertions(+), 48 deletions(-) create mode 100644 go/internal/controller/translator/agent/testdata/inputs/agent_with_nested_apikey_modelconfig.yaml create mode 100644 go/internal/controller/translator/agent/testdata/outputs/agent_with_nested_apikey_modelconfig.json diff --git a/go/api/v1alpha2/modelconfig_types.go b/go/api/v1alpha2/modelconfig_types.go index 4e9315730..417ffed9c 100644 --- a/go/api/v1alpha2/modelconfig_types.go +++ b/go/api/v1alpha2/modelconfig_types.go @@ -210,6 +210,19 @@ type OllamaConfig struct { type GeminiConfig struct{} +// APIKeyConfig contains API key secret configuration for model provider authentication. +type APIKeyConfig struct { + // SecretRef is the name of the Kubernetes Secret containing the API key. + // The Secret must be in the same namespace as the ModelConfig. + // +optional + SecretRef string `json:"secretRef,omitempty"` + + // SecretKey is the key within the Secret that contains the API key data. + // Required when SecretRef is set. + // +optional + SecretKey string `json:"secretKey,omitempty"` +} + // TLSConfig contains TLS/SSL configuration options for model provider connections. // This enables agents to connect to internal LiteLLM gateways or other providers // that use self-signed certificates or custom certificate authorities. @@ -255,6 +268,9 @@ type TLSConfig struct { // +kubebuilder:validation:XValidation:message="provider.gemini must be nil if the provider is not Gemini",rule="!(has(self.gemini) && self.provider != 'Gemini')" // +kubebuilder:validation:XValidation:message="provider.geminiVertexAI must be nil if the provider is not GeminiVertexAI",rule="!(has(self.geminiVertexAI) && self.provider != 'GeminiVertexAI')" // +kubebuilder:validation:XValidation:message="provider.anthropicVertexAI must be nil if the provider is not AnthropicVertexAI",rule="!(has(self.anthropicVertexAI) && self.provider != 'AnthropicVertexAI')" +// +kubebuilder:validation:XValidation:message="cannot use both apiKey and deprecated apiKeySecret/apiKeySecretKey fields",rule="!(has(self.apiKey) && (has(self.apiKeySecret) || has(self.apiKeySecretKey)))" +// +kubebuilder:validation:XValidation:message="apiKey.secretRef must be set if apiKey.secretKey is set",rule="!(has(self.apiKey) && has(self.apiKey.secretKey) && size(self.apiKey.secretKey) > 0 && (!has(self.apiKey.secretRef) || size(self.apiKey.secretRef) == 0))" +// +kubebuilder:validation:XValidation:message="apiKey.secretKey must be set if apiKey.secretRef is set",rule="!(has(self.apiKey) && has(self.apiKey.secretRef) && size(self.apiKey.secretRef) > 0 && (!has(self.apiKey.secretKey) || size(self.apiKey.secretKey) == 0))" // +kubebuilder:validation:XValidation:message="apiKeySecret must be set if apiKeySecretKey is set",rule="!(has(self.apiKeySecretKey) && !has(self.apiKeySecret))" // +kubebuilder:validation:XValidation:message="apiKeySecretKey must be set if apiKeySecret is set",rule="!(has(self.apiKeySecret) && !has(self.apiKeySecretKey))" // +kubebuilder:validation:XValidation:message="caCertSecretKey requires caCertSecretRef",rule="!(has(self.tls) && has(self.tls.caCertSecretKey) && size(self.tls.caCertSecretKey) > 0 && (!has(self.tls.caCertSecretRef) || size(self.tls.caCertSecretRef) == 0))" @@ -263,13 +279,19 @@ type TLSConfig struct { type ModelConfigSpec struct { Model string `json:"model"` - // The name of the secret that contains the API key. Must be a reference to the name of a secret in the same namespace as the referencing ModelConfig + // APIKey contains the API key secret configuration. // +optional - APIKeySecret string `json:"apiKeySecret"` + APIKey *APIKeyConfig `json:"apiKey,omitempty"` - // The key in the secret that contains the API key + // Deprecated: Use APIKey.SecretRef instead. + // The name of the secret that contains the API key. Must be a reference to the name of a secret in the same namespace as the referencing ModelConfig. // +optional - APIKeySecretKey string `json:"apiKeySecretKey"` + APIKeySecret string `json:"apiKeySecret,omitempty"` + + // Deprecated: Use APIKey.SecretKey instead. + // The key in the secret that contains the API key. + // +optional + APIKeySecretKey string `json:"apiKeySecretKey,omitempty"` // +optional DefaultHeaders map[string]string `json:"defaultHeaders,omitempty"` @@ -346,6 +368,23 @@ type ModelConfigList struct { Items []ModelConfig `json:"items"` } +// GetAPIKeySecretRef returns the API key secret name and key, supporting both the new nested +// structure (APIKey) and the deprecated flat fields (APIKeySecret/APIKeySecretKey). +// The new nested structure takes precedence if set. +func (s *ModelConfigSpec) GetAPIKeySecretRef() (secretName, secretKey string) { + if s.APIKey != nil && s.APIKey.SecretRef != "" { + return s.APIKey.SecretRef, s.APIKey.SecretKey + } + // Fall back to deprecated fields + return s.APIKeySecret, s.APIKeySecretKey +} + +// IsUsingDeprecatedAPIKeyFields returns true if the deprecated APIKeySecret/APIKeySecretKey +// fields are being used instead of the new nested APIKey structure. +func (s *ModelConfigSpec) IsUsingDeprecatedAPIKeyFields() bool { + return (s.APIKeySecret != "" || s.APIKeySecretKey != "") && (s.APIKey == nil || s.APIKey.SecretRef == "") +} + func init() { SchemeBuilder.Register(&ModelConfig{}, &ModelConfigList{}) } diff --git a/go/api/v1alpha2/zz_generated.deepcopy.go b/go/api/v1alpha2/zz_generated.deepcopy.go index 87e49ab21..26ebfcb80 100644 --- a/go/api/v1alpha2/zz_generated.deepcopy.go +++ b/go/api/v1alpha2/zz_generated.deepcopy.go @@ -48,6 +48,21 @@ func (in *A2AConfig) DeepCopy() *A2AConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *APIKeyConfig) DeepCopyInto(out *APIKeyConfig) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new APIKeyConfig. +func (in *APIKeyConfig) DeepCopy() *APIKeyConfig { + if in == nil { + return nil + } + out := new(APIKeyConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Agent) DeepCopyInto(out *Agent) { *out = *in @@ -512,6 +527,11 @@ func (in *ModelConfigList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ModelConfigSpec) DeepCopyInto(out *ModelConfigSpec) { *out = *in + if in.APIKey != nil { + in, out := &in.APIKey, &out.APIKey + *out = new(APIKeyConfig) + **out = **in + } if in.DefaultHeaders != nil { in, out := &in.DefaultHeaders, &out.DefaultHeaders *out = make(map[string]string, len(*in)) diff --git a/go/config/crd/bases/kagent.dev_modelconfigs.yaml b/go/config/crd/bases/kagent.dev_modelconfigs.yaml index 74153526d..9ae883244 100644 --- a/go/config/crd/bases/kagent.dev_modelconfigs.yaml +++ b/go/config/crd/bases/kagent.dev_modelconfigs.yaml @@ -430,13 +430,29 @@ spec: - location - projectID type: object + apiKey: + description: APIKey contains the API key secret configuration. + properties: + secretKey: + description: |- + SecretKey is the key within the Secret that contains the API key data. + Required when SecretRef is set. + type: string + secretRef: + description: |- + SecretRef is the name of the Kubernetes Secret containing the API key. + The Secret must be in the same namespace as the ModelConfig. + type: string + type: object apiKeySecret: - description: The name of the secret that contains the API key. Must - be a reference to the name of a secret in the same namespace as - the referencing ModelConfig + description: |- + Deprecated: Use APIKey.SecretRef instead. + The name of the secret that contains the API key. Must be a reference to the name of a secret in the same namespace as the referencing ModelConfig. type: string apiKeySecretKey: - description: The key in the secret that contains the API key + description: |- + Deprecated: Use APIKey.SecretKey instead. + The key in the secret that contains the API key. type: string azureOpenAI: description: Azure OpenAI-specific configuration @@ -636,6 +652,17 @@ spec: - message: provider.anthropicVertexAI must be nil if the provider is not AnthropicVertexAI rule: '!(has(self.anthropicVertexAI) && self.provider != ''AnthropicVertexAI'')' + - message: cannot use both apiKey and deprecated apiKeySecret/apiKeySecretKey + fields + rule: '!(has(self.apiKey) && (has(self.apiKeySecret) || has(self.apiKeySecretKey)))' + - message: apiKey.secretRef must be set if apiKey.secretKey is set + rule: '!(has(self.apiKey) && has(self.apiKey.secretKey) && size(self.apiKey.secretKey) + > 0 && (!has(self.apiKey.secretRef) || size(self.apiKey.secretRef) + == 0))' + - message: apiKey.secretKey must be set if apiKey.secretRef is set + rule: '!(has(self.apiKey) && has(self.apiKey.secretRef) && size(self.apiKey.secretRef) + > 0 && (!has(self.apiKey.secretKey) || size(self.apiKey.secretKey) + == 0))' - message: apiKeySecret must be set if apiKeySecretKey is set rule: '!(has(self.apiKeySecretKey) && !has(self.apiKeySecret))' - message: apiKeySecretKey must be set if apiKeySecret is set diff --git a/go/internal/controller/modelconfig_controller.go b/go/internal/controller/modelconfig_controller.go index afdbe8396..687a440d0 100644 --- a/go/internal/controller/modelconfig_controller.go +++ b/go/internal/controller/modelconfig_controller.go @@ -118,8 +118,8 @@ func modelReferencesSecret(model *v1alpha2.ModelConfig, secretObj types.Namespac return false } - // check if secret is referenced as an APIKey - if model.Spec.APIKeySecret != "" && model.Spec.APIKeySecret == secretObj.Name { + apiKeySecretName, _ := model.Spec.GetAPIKeySecretRef() + if apiKeySecretName != "" && apiKeySecretName == secretObj.Name { return true } diff --git a/go/internal/controller/reconciler/reconciler.go b/go/internal/controller/reconciler/reconciler.go index 3c5c76578..5a89f7f4e 100644 --- a/go/internal/controller/reconciler/reconciler.go +++ b/go/internal/controller/reconciler/reconciler.go @@ -225,13 +225,20 @@ func (a *kagentReconciler) ReconcileKagentModelConfig(ctx context.Context, req c var err error var secrets []secretRef - // check for api key secret - if modelConfig.Spec.APIKeySecret != "" { + if modelConfig.Spec.IsUsingDeprecatedAPIKeyFields() { + reconcileLog.Info( + "DEPRECATION WARNING: apiKeySecret and apiKeySecretKey fields are deprecated, use apiKey.secretRef and apiKey.secretKey instead", + "modelConfig", utils.GetObjectRef(modelConfig), + ) + } + + apiKeySecretName, _ := modelConfig.Spec.GetAPIKeySecretRef() + if apiKeySecretName != "" { secret := &corev1.Secret{} - namespacedName := types.NamespacedName{Namespace: modelConfig.Namespace, Name: modelConfig.Spec.APIKeySecret} + namespacedName := types.NamespacedName{Namespace: modelConfig.Namespace, Name: apiKeySecretName} if kubeErr := a.kube.Get(ctx, namespacedName, secret); kubeErr != nil { - err = multierror.Append(err, fmt.Errorf("failed to get secret %s: %v", modelConfig.Spec.APIKeySecret, kubeErr)) + err = multierror.Append(err, fmt.Errorf("failed to get secret %s: %v", apiKeySecretName, kubeErr)) } else { secrets = append(secrets, secretRef{ NamespacedName: namespacedName, diff --git a/go/internal/controller/translator/agent/adk_api_translator.go b/go/internal/controller/translator/agent/adk_api_translator.go index a215ad383..79c86468c 100644 --- a/go/internal/controller/translator/agent/adk_api_translator.go +++ b/go/internal/controller/translator/agent/adk_api_translator.go @@ -654,15 +654,16 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC switch model.Spec.Provider { case v1alpha2.ModelProviderOpenAI: - if model.Spec.APIKeySecret != "" { + apiKeySecretName, apiKeySecretKey := model.Spec.GetAPIKeySecretRef() + if apiKeySecretName != "" { modelDeploymentData.EnvVars = append(modelDeploymentData.EnvVars, corev1.EnvVar{ Name: "OPENAI_API_KEY", ValueFrom: &corev1.EnvVarSource{ SecretKeyRef: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ - Name: model.Spec.APIKeySecret, + Name: apiKeySecretName, }, - Key: model.Spec.APIKeySecretKey, + Key: apiKeySecretKey, }, }, }) @@ -709,15 +710,16 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC } return openai, modelDeploymentData, secretHashBytes, nil case v1alpha2.ModelProviderAnthropic: - if model.Spec.APIKeySecret != "" { + apiKeySecretName, apiKeySecretKey := model.Spec.GetAPIKeySecretRef() + if apiKeySecretName != "" { modelDeploymentData.EnvVars = append(modelDeploymentData.EnvVars, corev1.EnvVar{ Name: "ANTHROPIC_API_KEY", ValueFrom: &corev1.EnvVarSource{ SecretKeyRef: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ - Name: model.Spec.APIKeySecret, + Name: apiKeySecretName, }, - Key: model.Spec.APIKeySecretKey, + Key: apiKeySecretKey, }, }, }) @@ -739,14 +741,15 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC if model.Spec.AzureOpenAI == nil { return nil, nil, nil, fmt.Errorf("AzureOpenAI model config is required") } + apiKeySecretName, apiKeySecretKey := model.Spec.GetAPIKeySecretRef() modelDeploymentData.EnvVars = append(modelDeploymentData.EnvVars, corev1.EnvVar{ Name: "AZURE_OPENAI_API_KEY", ValueFrom: &corev1.EnvVarSource{ SecretKeyRef: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ - Name: model.Spec.APIKeySecret, + Name: apiKeySecretName, }, - Key: model.Spec.APIKeySecretKey, + Key: apiKeySecretKey, }, }, }) @@ -794,16 +797,17 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC Name: "GOOGLE_GENAI_USE_VERTEXAI", Value: "true", }) - if model.Spec.APIKeySecret != "" { + apiKeySecretName, apiKeySecretKey := model.Spec.GetAPIKeySecretRef() + if apiKeySecretName != "" { modelDeploymentData.EnvVars = append(modelDeploymentData.EnvVars, corev1.EnvVar{ Name: "GOOGLE_APPLICATION_CREDENTIALS", - Value: "/creds/" + model.Spec.APIKeySecretKey, + Value: "/creds/" + apiKeySecretKey, }) modelDeploymentData.Volumes = append(modelDeploymentData.Volumes, corev1.Volume{ Name: googleCredsVolumeName, VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ - SecretName: model.Spec.APIKeySecret, + SecretName: apiKeySecretName, }, }, }) @@ -834,16 +838,17 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC Name: "GOOGLE_CLOUD_LOCATION", Value: model.Spec.AnthropicVertexAI.Location, }) - if model.Spec.APIKeySecret != "" { + apiKeySecretName, apiKeySecretKey := model.Spec.GetAPIKeySecretRef() + if apiKeySecretName != "" { modelDeploymentData.EnvVars = append(modelDeploymentData.EnvVars, corev1.EnvVar{ Name: "GOOGLE_APPLICATION_CREDENTIALS", - Value: "/creds/" + model.Spec.APIKeySecretKey, + Value: "/creds/" + apiKeySecretKey, }) modelDeploymentData.Volumes = append(modelDeploymentData.Volumes, corev1.Volume{ Name: googleCredsVolumeName, VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ - SecretName: model.Spec.APIKeySecret, + SecretName: apiKeySecretName, }, }, }) @@ -881,14 +886,15 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC return ollama, modelDeploymentData, secretHashBytes, nil case v1alpha2.ModelProviderGemini: + apiKeySecretName, apiKeySecretKey := model.Spec.GetAPIKeySecretRef() modelDeploymentData.EnvVars = append(modelDeploymentData.EnvVars, corev1.EnvVar{ Name: "GOOGLE_API_KEY", ValueFrom: &corev1.EnvVarSource{ SecretKeyRef: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ - Name: model.Spec.APIKeySecret, + Name: apiKeySecretName, }, - Key: model.Spec.APIKeySecretKey, + Key: apiKeySecretKey, }, }, }) diff --git a/go/internal/controller/translator/agent/testdata/inputs/agent_with_nested_apikey_modelconfig.yaml b/go/internal/controller/translator/agent/testdata/inputs/agent_with_nested_apikey_modelconfig.yaml new file mode 100644 index 000000000..a92c8f863 --- /dev/null +++ b/go/internal/controller/translator/agent/testdata/inputs/agent_with_nested_apikey_modelconfig.yaml @@ -0,0 +1,28 @@ +operation: translateAgent +targetObject: test-agent +namespace: default +objects: + - apiVersion: kagent.dev/v1alpha2 + kind: ModelConfig + metadata: + name: nested-apikey-model + namespace: default + spec: + model: gpt-4 + provider: OpenAI + apiKey: + secretRef: my-nested-secret + secretKey: my-nested-key + openAI: + baseUrl: https://api.openai.com/v1 + temperature: "0.7" + - apiVersion: kagent.dev/v1alpha2 + kind: Agent + metadata: + name: test-agent + namespace: default + spec: + type: Declarative + declarative: + modelConfig: nested-apikey-model + systemMessage: "You are a test agent." diff --git a/go/internal/controller/translator/agent/testdata/outputs/agent_with_nested_apikey_modelconfig.json b/go/internal/controller/translator/agent/testdata/outputs/agent_with_nested_apikey_modelconfig.json new file mode 100644 index 000000000..603356cfc --- /dev/null +++ b/go/internal/controller/translator/agent/testdata/outputs/agent_with_nested_apikey_modelconfig.json @@ -0,0 +1,292 @@ +{ + "agentCard": { + "capabilities": { + "pushNotifications": false, + "stateTransitionHistory": true, + "streaming": true + }, + "defaultInputModes": [ + "text" + ], + "defaultOutputModes": [ + "text" + ], + "description": "", + "name": "test_agent", + "skills": null, + "url": "http://test-agent.default:8080", + "version": "" + }, + "config": { + "description": "", + "http_tools": null, + "instruction": "You are a test agent.", + "model": { + "base_url": "https://api.openai.com/v1", + "model": "gpt-4", + "temperature": 0.7, + "type": "openai" + }, + "remote_agents": null, + "sse_tools": null + }, + "manifest": [ + { + "apiVersion": "v1", + "kind": "Secret", + "metadata": { + "labels": { + "app": "kagent", + "app.kubernetes.io/managed-by": "kagent", + "app.kubernetes.io/name": "test-agent", + "app.kubernetes.io/part-of": "kagent", + "kagent": "test-agent" + }, + "name": "test-agent", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "kagent.dev/v1alpha2", + "blockOwnerDeletion": true, + "controller": true, + "kind": "Agent", + "name": "test-agent", + "uid": "" + } + ] + }, + "stringData": { + "agent-card.json": "{\"name\":\"test_agent\",\"description\":\"\",\"url\":\"http://test-agent.default:8080\",\"version\":\"\",\"capabilities\":{\"streaming\":true,\"pushNotifications\":false,\"stateTransitionHistory\":true},\"defaultInputModes\":[\"text\"],\"defaultOutputModes\":[\"text\"],\"skills\":[]}", + "config.json": "{\"model\":{\"type\":\"openai\",\"model\":\"gpt-4\",\"base_url\":\"https://api.openai.com/v1\",\"temperature\":0.7},\"description\":\"\",\"instruction\":\"You are a test agent.\",\"http_tools\":null,\"sse_tools\":null,\"remote_agents\":null}" + } + }, + { + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": { + "labels": { + "app": "kagent", + "app.kubernetes.io/managed-by": "kagent", + "app.kubernetes.io/name": "test-agent", + "app.kubernetes.io/part-of": "kagent", + "kagent": "test-agent" + }, + "name": "test-agent", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "kagent.dev/v1alpha2", + "blockOwnerDeletion": true, + "controller": true, + "kind": "Agent", + "name": "test-agent", + "uid": "" + } + ] + } + }, + { + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "labels": { + "app": "kagent", + "app.kubernetes.io/managed-by": "kagent", + "app.kubernetes.io/name": "test-agent", + "app.kubernetes.io/part-of": "kagent", + "kagent": "test-agent" + }, + "name": "test-agent", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "kagent.dev/v1alpha2", + "blockOwnerDeletion": true, + "controller": true, + "kind": "Agent", + "name": "test-agent", + "uid": "" + } + ] + }, + "spec": { + "selector": { + "matchLabels": { + "app": "kagent", + "kagent": "test-agent" + } + }, + "strategy": { + "rollingUpdate": { + "maxSurge": 1, + "maxUnavailable": 0 + }, + "type": "RollingUpdate" + }, + "template": { + "metadata": { + "annotations": { + "kagent.dev/config-hash": "14360832279022375846" + }, + "labels": { + "app": "kagent", + "app.kubernetes.io/managed-by": "kagent", + "app.kubernetes.io/name": "test-agent", + "app.kubernetes.io/part-of": "kagent", + "kagent": "test-agent" + } + }, + "spec": { + "containers": [ + { + "args": [ + "--host", + "0.0.0.0", + "--port", + "8080", + "--filepath", + "/config" + ], + "env": [ + { + "name": "OPENAI_API_KEY", + "valueFrom": { + "secretKeyRef": { + "key": "my-nested-key", + "name": "my-nested-secret" + } + } + }, + { + "name": "KAGENT_NAMESPACE", + "valueFrom": { + "fieldRef": { + "fieldPath": "metadata.namespace" + } + } + }, + { + "name": "KAGENT_NAME", + "valueFrom": { + "fieldRef": { + "fieldPath": "spec.serviceAccountName" + } + } + }, + { + "name": "KAGENT_URL", + "value": "http://kagent-controller.kagent:8083" + } + ], + "image": "cr.kagent.dev/kagent-dev/kagent/app:dev", + "imagePullPolicy": "IfNotPresent", + "name": "kagent", + "ports": [ + { + "containerPort": 8080, + "name": "http" + } + ], + "readinessProbe": { + "httpGet": { + "path": "/health", + "port": "http" + }, + "initialDelaySeconds": 15, + "periodSeconds": 15, + "timeoutSeconds": 15 + }, + "resources": { + "limits": { + "cpu": "2", + "memory": "1Gi" + }, + "requests": { + "cpu": "100m", + "memory": "384Mi" + } + }, + "volumeMounts": [ + { + "mountPath": "/config", + "name": "config" + }, + { + "mountPath": "/var/run/secrets/tokens", + "name": "kagent-token" + } + ] + } + ], + "serviceAccountName": "test-agent", + "volumes": [ + { + "name": "config", + "secret": { + "secretName": "test-agent" + } + }, + { + "name": "kagent-token", + "projected": { + "sources": [ + { + "serviceAccountToken": { + "audience": "kagent", + "expirationSeconds": 3600, + "path": "kagent-token" + } + } + ] + } + } + ] + } + } + }, + "status": {} + }, + { + "apiVersion": "v1", + "kind": "Service", + "metadata": { + "labels": { + "app": "kagent", + "app.kubernetes.io/managed-by": "kagent", + "app.kubernetes.io/name": "test-agent", + "app.kubernetes.io/part-of": "kagent", + "kagent": "test-agent" + }, + "name": "test-agent", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "kagent.dev/v1alpha2", + "blockOwnerDeletion": true, + "controller": true, + "kind": "Agent", + "name": "test-agent", + "uid": "" + } + ] + }, + "spec": { + "ports": [ + { + "name": "http", + "port": 8080, + "targetPort": 8080 + } + ], + "selector": { + "app": "kagent", + "kagent": "test-agent" + }, + "type": "ClusterIP" + }, + "status": { + "loadBalancer": {} + } + } + ] +} \ No newline at end of file diff --git a/go/internal/httpserver/handlers/modelconfig.go b/go/internal/httpserver/handlers/modelconfig.go index 2c5c6ceee..72fd29044 100644 --- a/go/internal/httpserver/handlers/modelconfig.go +++ b/go/internal/httpserver/handlers/modelconfig.go @@ -61,14 +61,16 @@ func (h *ModelConfigHandler) HandleListModelConfigs(w ErrorResponseWriter, r *ht FlattenStructToMap(config.Spec.Ollama, modelParams) } + apiKeySecretName, apiKeySecretKey := config.Spec.GetAPIKeySecretRef() responseItem := api.ModelConfigResponse{ Ref: common.GetObjectRef(&config), ProviderName: string(config.Spec.Provider), Model: config.Spec.Model, - APIKeySecret: config.Spec.APIKeySecret, - APIKeySecretKey: config.Spec.APIKeySecretKey, ModelParams: modelParams, TLS: config.Spec.TLS, + APIKey: config.Spec.APIKey, + APIKeySecret: apiKeySecretName, + APIKeySecretKey: apiKeySecretKey, } configs = append(configs, responseItem) } @@ -144,14 +146,16 @@ func (h *ModelConfigHandler) HandleGetModelConfig(w ErrorResponseWriter, r *http FlattenStructToMap(modelConfig.Spec.Ollama, modelParams) } + apiKeySecretName, apiKeySecretKey := modelConfig.Spec.GetAPIKeySecretRef() responseItem := api.ModelConfigResponse{ Ref: common.GetObjectRef(modelConfig), ProviderName: string(modelConfig.Spec.Provider), Model: modelConfig.Spec.Model, - APIKeySecret: modelConfig.Spec.APIKeySecret, - APIKeySecretKey: modelConfig.Spec.APIKeySecretKey, ModelParams: modelParams, TLS: modelConfig.Spec.TLS, + APIKey: modelConfig.Spec.APIKey, + APIKeySecret: apiKeySecretName, + APIKeySecretKey: apiKeySecretKey, } log.Info("Successfully retrieved and formatted ModelConfig") @@ -239,12 +243,13 @@ func (h *ModelConfigHandler) HandleCreateModelConfig(w ErrorResponseWriter, r *h Provider: providerTypeEnum, } - // Set secret references if needed, but don't create secret yet if providerTypeEnum != v1alpha2.ModelProviderOllama && req.APIKey != "" { secretName := modelConfigRef.Name secretKey := fmt.Sprintf("%s_API_KEY", strings.ToUpper(req.Provider.Type)) - modelConfigSpec.APIKeySecret = secretName - modelConfigSpec.APIKeySecretKey = secretKey + modelConfigSpec.APIKey = &v1alpha2.APIKeyConfig{ + SecretRef: secretName, + SecretKey: secretKey, + } } modelConfig := &v1alpha2.ModelConfig{ @@ -423,6 +428,7 @@ func (h *ModelConfigHandler) HandleUpdateModelConfig(w ErrorResponseWriter, r *h modelConfig.Spec = v1alpha2.ModelConfigSpec{ Model: req.Model, Provider: v1alpha2.ModelProvider(req.Provider.Type), + APIKey: modelConfig.Spec.APIKey, APIKeySecret: modelConfig.Spec.APIKeySecret, APIKeySecretKey: modelConfig.Spec.APIKeySecretKey, OpenAI: nil, @@ -434,15 +440,15 @@ func (h *ModelConfigHandler) HandleUpdateModelConfig(w ErrorResponseWriter, r *h AnthropicVertexAI: nil, } - // --- Update Secret if API Key is provided (and not Ollama) --- shouldUpdateSecret := req.APIKey != nil && *req.APIKey != "" && modelConfig.Spec.Provider != v1alpha2.ModelProviderOllama if shouldUpdateSecret { log.V(1).Info("Updating API key secret") + _, apiKeySecretKey := modelConfig.Spec.GetAPIKeySecretRef() err := createOrUpdateSecretWithOwnerReference( r.Context(), h.KubeClient, - map[string]string{modelConfig.Spec.APIKeySecretKey: *req.APIKey}, + map[string]string{apiKeySecretKey: *req.APIKey}, modelConfig, ) if err != nil { @@ -538,14 +544,16 @@ func (h *ModelConfigHandler) HandleUpdateModelConfig(w ErrorResponseWriter, r *h FlattenStructToMap(modelConfig.Spec.Ollama, updatedParams) } + apiKeySecretName, apiKeySecretKey := modelConfig.Spec.GetAPIKeySecretRef() responseItem := api.ModelConfigResponse{ Ref: common.GetObjectRef(modelConfig), ProviderName: string(modelConfig.Spec.Provider), - APIKeySecret: modelConfig.Spec.APIKeySecret, - APIKeySecretKey: modelConfig.Spec.APIKeySecretKey, Model: modelConfig.Spec.Model, ModelParams: updatedParams, TLS: modelConfig.Spec.TLS, + APIKey: modelConfig.Spec.APIKey, + APIKeySecret: apiKeySecretName, + APIKeySecretKey: apiKeySecretKey, } log.V(1).Info("Successfully updated ModelConfig") diff --git a/go/internal/httpserver/handlers/modelconfig_test.go b/go/internal/httpserver/handlers/modelconfig_test.go index daa80977d..a8dee439f 100644 --- a/go/internal/httpserver/handlers/modelconfig_test.go +++ b/go/internal/httpserver/handlers/modelconfig_test.go @@ -90,6 +90,54 @@ func TestModelConfigHandler(t *testing.T) { assert.Equal(t, "OPENAI_API_KEY", config.APIKeySecretKey) }) + t.Run("Success_NestedAPIKey", func(t *testing.T) { + handler, kubeClient, responseRecorder := setupHandler() + + // Create test model config with nested API key + modelConfig := &v1alpha2.ModelConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-config-nested", + Namespace: "default", + }, + Spec: v1alpha2.ModelConfigSpec{ + Model: "gpt-4", + Provider: v1alpha2.ModelProviderOpenAI, + APIKey: &v1alpha2.APIKeyConfig{ + SecretRef: "nested-secret", + SecretKey: "NESTED_KEY", + }, + OpenAI: &v1alpha2.OpenAIConfig{ + BaseURL: "https://api.openai.com/v1", + }, + }, + } + + err := kubeClient.Create(context.Background(), modelConfig) + require.NoError(t, err) + + req := httptest.NewRequest("GET", "/api/modelconfigs/", nil) + req = setUser(req, "test-user") + handler.HandleListModelConfigs(responseRecorder, req) + + assert.Equal(t, http.StatusOK, responseRecorder.Code) + + var configs api.StandardResponse[[]api.ModelConfigResponse] + err = json.Unmarshal(responseRecorder.Body.Bytes(), &configs) + require.NoError(t, err) + assert.Len(t, configs.Data, 1) + + // Verify model config response + config := configs.Data[0] + assert.Equal(t, "default/test-config-nested", config.Ref) + // Verify new field is populated + require.NotNil(t, config.APIKey) + assert.Equal(t, "nested-secret", config.APIKey.SecretRef) + assert.Equal(t, "NESTED_KEY", config.APIKey.SecretKey) + // Verify backward compatibility fields are also populated + assert.Equal(t, "nested-secret", config.APIKeySecret) + assert.Equal(t, "NESTED_KEY", config.APIKeySecretKey) + }) + t.Run("EmptyList", func(t *testing.T) { handler, _, responseRecorder := setupHandler() @@ -138,6 +186,10 @@ func TestModelConfigHandler(t *testing.T) { assert.Equal(t, "default", config.Data.Namespace) assert.Equal(t, v1alpha2.ModelProviderOpenAI, config.Data.Spec.Provider) assert.Equal(t, "gpt-4", config.Data.Spec.Model) + // Verify API Key is set in the nested structure + require.NotNil(t, config.Data.Spec.APIKey) + assert.Equal(t, "test-config", config.Data.Spec.APIKey.SecretRef) + assert.Equal(t, "OPENAI_API_KEY", config.Data.Spec.APIKey.SecretKey) }) t.Run("Success_Anthropic", func(t *testing.T) { diff --git a/go/pkg/client/api/types.go b/go/pkg/client/api/types.go index a3a4b0e82..3655aba59 100644 --- a/go/pkg/client/api/types.go +++ b/go/pkg/client/api/types.go @@ -43,13 +43,15 @@ type VersionResponse struct { // ModelConfigResponse represents a model configuration response type ModelConfigResponse struct { - Ref string `json:"ref"` - ProviderName string `json:"providerName"` - Model string `json:"model"` - APIKeySecret string `json:"apiKeySecret"` - APIKeySecretKey string `json:"apiKeySecretKey"` - ModelParams map[string]any `json:"modelParams"` - TLS *v1alpha2.TLSConfig `json:"tls,omitempty"` + Ref string `json:"ref"` + ProviderName string `json:"providerName"` + Model string `json:"model"` + APIKey *v1alpha2.APIKeyConfig `json:"apiKey,omitempty"` + ModelParams map[string]any `json:"modelParams"` + TLS *v1alpha2.TLSConfig `json:"tls,omitempty"` + // Deprecated: Use APIKey instead. These fields are kept for backwards compatibility. + APIKeySecret string `json:"apiKeySecret,omitempty"` + APIKeySecretKey string `json:"apiKeySecretKey,omitempty"` } // CreateModelConfigRequest represents a request to create a model configuration