diff --git a/api/v1alpha1/mcpserver_types.go b/api/v1alpha1/mcpserver_types.go index d6f94e4..174e903 100644 --- a/api/v1alpha1/mcpserver_types.go +++ b/api/v1alpha1/mcpserver_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1alpha1 import ( + "encoding/json" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -215,7 +217,7 @@ type MCPServerDeployment struct { // Env defines the environment variables to set in the container. // +optional - Env map[string]string `json:"env,omitempty"` + Env map[string]EnvVarCfg `json:"env,omitempty"` // SecretRefs defines the list of Kubernetes secrets to reference. // These secrets will be mounted as volumes to the MCP server container. @@ -247,6 +249,39 @@ type MCPServerDeployment struct { ServiceAccount *ServiceAccountConfig `json:"serviceAccount,omitempty"` } +// EnvVarCfg allows specifying either a literal value or a reference to a source for the environment variable. +type EnvVarCfg struct { + // Value contains the value for the environment variable. + // Defaults to "" if not set. + // +optional + Value string `json:"value,omitempty"` + + // ValueFrom specifies a source the value of this EnvVar to come from. + // +optional + ValueFrom *corev1.EnvVarSource `json:"valueFrom,omitempty"` +} + +// UnmarshalJSON implements json.Unmarshaler for EnvVarCfg. +// It allows unmarshalling a plain string into the Value field, or an object into Value/ValueFrom. +func (e *EnvVarCfg) UnmarshalJSON(data []byte) error { + // Try to unmarshal as a string + var str string + if err := json.Unmarshal(data, &str); err == nil { + e.Value = str + return nil + } + + // If not a string, try to unmarshal as an object + type rawEnvVarCfg EnvVarCfg // Use a new type to avoid infinite recursion + var raw rawEnvVarCfg + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + e.Value = raw.Value + e.ValueFrom = raw.ValueFrom + return nil +} + // InitContainerConfig defines the configuration for the init container. type InitContainerConfig struct { // Image defines the full image reference for the init container. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index dcc88d1..df9656c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -26,6 +26,26 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EnvVarCfg) DeepCopyInto(out *EnvVarCfg) { + *out = *in + if in.ValueFrom != nil { + in, out := &in.ValueFrom, &out.ValueFrom + *out = new(corev1.EnvVarSource) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EnvVarCfg. +func (in *EnvVarCfg) DeepCopy() *EnvVarCfg { + if in == nil { + return nil + } + out := new(EnvVarCfg) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HTTPTransport) DeepCopyInto(out *HTTPTransport) { *out = *in @@ -93,9 +113,9 @@ func (in *MCPServerDeployment) DeepCopyInto(out *MCPServerDeployment) { } if in.Env != nil { in, out := &in.Env, &out.Env - *out = make(map[string]string, len(*in)) + *out = make(map[string]EnvVarCfg, len(*in)) for key, val := range *in { - (*out)[key] = val + (*out)[key] = *val.DeepCopy() } } if in.SecretRefs != nil { diff --git a/config/crd/bases/kagent.dev_mcpservers.yaml b/config/crd/bases/kagent.dev_mcpservers.yaml index 4459107..a66be8d 100644 --- a/config/crd/bases/kagent.dev_mcpservers.yaml +++ b/config/crd/bases/kagent.dev_mcpservers.yaml @@ -84,7 +84,109 @@ spec: type: array env: additionalProperties: - type: string + description: EnvVarCfg allows specifying either a literal value + or a reference to a source for the environment variable. + properties: + value: + description: |- + Value contains the value for the environment variable. + Defaults to "" if not set. + type: string + valueFrom: + description: ValueFrom specifies a source the value of this + EnvVar to come from. + properties: + configMapKeyRef: + description: Selects a key of a ConfigMap. + properties: + key: + description: The key to select. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the ConfigMap or its + key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + fieldRef: + description: |- + Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['']`, `metadata.annotations['']`, + spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, status.podIPs. + properties: + apiVersion: + description: Version of the schema the FieldPath + is written in terms of, defaults to "v1". + type: string + fieldPath: + description: Path of the field to select in the + specified API version. + type: string + required: + - fieldPath + type: object + x-kubernetes-map-type: atomic + resourceFieldRef: + description: |- + Selects a resource of the container: only resources limits and requests + (limits.cpu, limits.memory, limits.ephemeral-storage, requests.cpu, requests.memory and requests.ephemeral-storage) are currently supported. + properties: + containerName: + description: 'Container name: required for volumes, + optional for env vars' + type: string + divisor: + anyOf: + - type: integer + - type: string + description: Specifies the output format of the + exposed resources, defaults to "1" + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + resource: + description: 'Required: resource to select' + type: string + required: + - resource + type: object + x-kubernetes-map-type: atomic + secretKeyRef: + description: Selects a key of a secret in the pod's + namespace + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its key + must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: object description: Env defines the environment variables to set in the container. type: object diff --git a/helm/kmcp-crds/templates/mcpserver-crd.yaml b/helm/kmcp-crds/templates/mcpserver-crd.yaml index a7a4928..5288f34 100644 --- a/helm/kmcp-crds/templates/mcpserver-crd.yaml +++ b/helm/kmcp-crds/templates/mcpserver-crd.yaml @@ -84,7 +84,109 @@ spec: type: array env: additionalProperties: - type: string + description: EnvVarCfg allows specifying either a literal value + or a reference to a source for the environment variable. + properties: + value: + description: |- + Value contains the value for the environment variable. + Defaults to "" if not set. + type: string + valueFrom: + description: ValueFrom specifies a source the value of this + EnvVar to come from. + properties: + configMapKeyRef: + description: Selects a key of a ConfigMap. + properties: + key: + description: The key to select. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the ConfigMap or its + key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + fieldRef: + description: |- + Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['']`, `metadata.annotations['']`, + spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, status.podIPs. + properties: + apiVersion: + description: Version of the schema the FieldPath + is written in terms of, defaults to "v1". + type: string + fieldPath: + description: Path of the field to select in the + specified API version. + type: string + required: + - fieldPath + type: object + x-kubernetes-map-type: atomic + resourceFieldRef: + description: |- + Selects a resource of the container: only resources limits and requests + (limits.cpu, limits.memory, limits.ephemeral-storage, requests.cpu, requests.memory and requests.ephemeral-storage) are currently supported. + properties: + containerName: + description: 'Container name: required for volumes, + optional for env vars' + type: string + divisor: + anyOf: + - type: integer + - type: string + description: Specifies the output format of the + exposed resources, defaults to "1" + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + resource: + description: 'Required: resource to select' + type: string + required: + - resource + type: object + x-kubernetes-map-type: atomic + secretKeyRef: + description: Selects a key of a secret in the pod's + namespace + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its key + must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: object description: Env defines the environment variables to set in the container. type: object diff --git a/pkg/cli/internal/commands/deploy.go b/pkg/cli/internal/commands/deploy.go index bdc1673..63d85ed 100644 --- a/pkg/cli/internal/commands/deploy.go +++ b/pkg/cli/internal/commands/deploy.go @@ -217,7 +217,7 @@ func runPackageDeploy(_ *cobra.Command, _ []string) error { Port: uint16(port), Cmd: packageManager, Args: deployArgs, - Env: envMap, + Env: toEnvVarCfgMap(envMap), SecretRefs: secretRefs, }, TransportType: getTransportType(), @@ -468,7 +468,7 @@ func generateMCPServer( Port: uint16(port), Cmd: command, Args: args, - Env: envVars, + Env: toEnvVarCfgMap(envVars), SecretRefs: secretRefs, }, TransportType: transportType, @@ -571,6 +571,15 @@ func parseEnvVars(envVars []string) map[string]string { return result } +// toEnvVarCfgMap converts a map[string]string to a map[string]v1alpha1.EnvVarCfg +func toEnvVarCfgMap(envMapString map[string]string) map[string]v1alpha1.EnvVarCfg { + result := make(map[string]v1alpha1.EnvVarCfg) + for k, v := range envMapString { + result[k] = v1alpha1.EnvVarCfg{Value: v} + } + return result +} + func applyToCluster(projectDir, yamlContent string, mcpServer *v1alpha1.MCPServer) error { fmt.Printf("🚀 Applying MCPServer to cluster...\n") diff --git a/pkg/controller/transportadapter/transportadapter_translator.go b/pkg/controller/transportadapter/transportadapter_translator.go index e16fa88..1de4fa6 100644 --- a/pkg/controller/transportadapter/transportadapter_translator.go +++ b/pkg/controller/transportadapter/transportadapter_translator.go @@ -102,6 +102,12 @@ func (t *transportAdapterTranslator) translateTransportAdapterDeployment( return nil, fmt.Errorf("image must be specified for MCPServer %s or the command must be 'uvx' or 'npx'", server.Name) } + // Convert EnvVarCfg to corev1.EnvVar for deployment + envVars, err := convertEnvVars(server.Spec.Deployment.Env) + if err != nil { + return nil, err + } + // Create environment variables from secrets for envFrom secretEnvFrom := t.createSecretEnvFrom(server.Spec.Deployment.SecretRefs) @@ -163,7 +169,7 @@ func (t *transportAdapterTranslator) translateTransportAdapterDeployment( "-f", "/config/local.yaml", }, - Env: convertEnvVars(server.Spec.Deployment.Env), + Env: envVars, EnvFrom: secretEnvFrom, VolumeMounts: append([]corev1.VolumeMount{ { @@ -209,7 +215,7 @@ func (t *transportAdapterTranslator) translateTransportAdapterDeployment( ImagePullPolicy: mainContainerPullPolicy, Command: cmd, Args: server.Spec.Deployment.Args, - Env: convertEnvVars(server.Spec.Deployment.Env), + Env: envVars, EnvFrom: secretEnvFrom, VolumeMounts: append([]corev1.VolumeMount{ { @@ -398,21 +404,50 @@ func (t *transportAdapterTranslator) createVolumeMounts( return volumeMounts } -func convertEnvVars(env map[string]string) []corev1.EnvVar { +func convertEnvVars(env map[string]v1alpha1.EnvVarCfg) ([]corev1.EnvVar, error) { if env == nil { - return nil + return nil, nil } envVars := make([]corev1.EnvVar, 0, len(env)) - for key, value := range env { - envVars = append(envVars, corev1.EnvVar{ - Name: key, - Value: value, - }) + for key, cfg := range env { + if cfg.Value != "" && cfg.ValueFrom != nil { + return nil, fmt.Errorf("environment variable %s cannot specify both value and valueFrom", key) + } + if cfg.Value != "" { + envVars = append(envVars, corev1.EnvVar{ + Name: key, + Value: cfg.Value, + }) + } else if cfg.ValueFrom != nil { + envVars = append(envVars, corev1.EnvVar{ + Name: key, + ValueFrom: cfg.ValueFrom, + }) + } } sort.Slice(envVars, func(i, j int) bool { return envVars[i].Name < envVars[j].Name }) - return envVars + return envVars, nil +} + +// createStdioEnvConfig creates the environment variable configuration for the StdioTargetSpec. +// It includes all EnvVarCfg entries, including those with ValueFrom. +// Note: The underlying transport adapter (agentgateway) may not support ValueFrom in its config. +func createStdioEnvConfig(env map[string]v1alpha1.EnvVarCfg) map[string]v1alpha1.EnvVarCfg { + if env == nil { + return nil + } + stdioEnvConfig := make(map[string]v1alpha1.EnvVarCfg) + for key, cfg := range env { + // Basic validation - this should ideally be handled by CRD validation + if cfg.Value != "" && cfg.ValueFrom != nil { + klog.Warningf("Environment variable %s in MCPServer.Spec.Deployment.Env specifies both value and valueFrom. Prioritizing valueFrom for Kubernetes Deployment, and including both for internal config. This may lead to unexpected behavior.", key) + } + // Pass through the EnvVarCfg as is, the generated struct expects this type + stdioEnvConfig[key] = cfg + } + return stdioEnvConfig } func (t *transportAdapterTranslator) translateTransportAdapterService( @@ -506,7 +541,7 @@ func (t *transportAdapterTranslator) translateTransportAdapterConfig(server *v1a mcpTarget.Stdio = &StdioTargetSpec{ Cmd: server.Spec.Deployment.Cmd, Args: server.Spec.Deployment.Args, - Env: server.Spec.Deployment.Env, + Env: createStdioEnvConfig(server.Spec.Deployment.Env), } case v1alpha1.TransportTypeHTTP: httpTransportConfig := server.Spec.HTTPTransport diff --git a/pkg/controller/transportadapter/transportadapter_types.go b/pkg/controller/transportadapter/transportadapter_types.go index 1898462..6fafdc1 100644 --- a/pkg/controller/transportadapter/transportadapter_types.go +++ b/pkg/controller/transportadapter/transportadapter_types.go @@ -4,6 +4,8 @@ package transportadapter import ( "net" "time" + + "github.com/kagent-dev/kmcp/api/v1alpha1" ) // ============================================================================ @@ -227,9 +229,9 @@ type SSETargetSpec struct { // StdioTargetSpec represents stdio target specification type StdioTargetSpec struct { - Cmd string `json:"cmd" yaml:"cmd"` - Args []string `json:"args,omitempty" yaml:"args,omitempty"` - Env map[string]string `json:"env,omitempty" yaml:"env,omitempty"` + Cmd string `json:"cmd" yaml:"cmd"` + Args []string `json:"args,omitempty" yaml:"args,omitempty"` + Env map[string]v1alpha1.EnvVarCfg `json:"env,omitempty" yaml:"env,omitempty"` } // OpenAPITargetSpec represents OpenAPI target specification