diff --git a/pkg/mcp/tools_config_envs.go b/pkg/mcp/tools_config_envs.go index a2d1fceefa..397b8c1eef 100644 --- a/pkg/mcp/tools_config_envs.go +++ b/pkg/mcp/tools_config_envs.go @@ -48,9 +48,21 @@ func (s *Server) configEnvsListHandler(ctx context.Context, r *mcp.CallToolReque // config_envs_add var configEnvsAddTool = &mcp.Tool{ - Name: "config_envs_add", - Title: "Config Environment Variables Add", - Description: "Adds an environment variable to a function's configuration.", + Name: "config_envs_add", + Title: "Config Environment Variables Add", + Description: `Adds an environment variable to a function's configuration. + +Supports six source types: + 1. Literal value: set Name and Value directly. + 2. Local env var: set Name and Value to "{{ env:LOCAL_VAR }}". + 3. Secret (all keys): set SecretName only — imports every key from the Secret as env vars. + 4. Secret (one key): set Name, SecretName, and SecretKey. + 5. ConfigMap (all): set ConfigMapName only — imports every key from the ConfigMap as env vars. + 6. ConfigMap (one key): set Name, ConfigMapName, and ConfigMapKey. + +When SecretName or ConfigMapName is provided, the tool constructs the +appropriate "{{ secret:… }}" or "{{ configMap:… }}" value template automatically. +Value is mutually exclusive with SecretName and ConfigMapName.`, Annotations: &mcp.ToolAnnotations{ Title: "Config Environment Variables Add", ReadOnlyHint: false, @@ -60,16 +72,67 @@ var configEnvsAddTool = &mcp.Tool{ } type ConfigEnvsAddInput struct { - Path string `json:"path" jsonschema:"required,Path to the function project directory"` - Name *string `json:"name,omitempty" jsonschema:"Name of the environment variable"` - Value *string `json:"value,omitempty" jsonschema:"Value of the environment variable"` - Verbose *bool `json:"verbose,omitempty" jsonschema:"Enable verbose logging output"` + Path string `json:"path" jsonschema:"required,Path to the function project directory"` + Name *string `json:"name,omitempty" jsonschema:"Name of the environment variable"` + Value *string `json:"value,omitempty" jsonschema:"Literal value or template expression (e.g. '{{ env:MY_VAR }}') for the environment variable"` + SecretName *string `json:"secretName,omitempty" jsonschema:"Name of the Kubernetes Secret to source the value from"` + SecretKey *string `json:"secretKey,omitempty" jsonschema:"Key within the Secret; omit to import all keys as env vars"` + ConfigMapName *string `json:"configMapName,omitempty" jsonschema:"Name of the Kubernetes ConfigMap to source the value from"` + ConfigMapKey *string `json:"configMapKey,omitempty" jsonschema:"Key within the ConfigMap; omit to import all keys as env vars"` + Verbose *bool `json:"verbose,omitempty" jsonschema:"Enable verbose logging output"` +} + +// validate returns an error for any illegal input combination before args are +// constructed and forwarded to the CLI. Character-set validation is deferred to +// func's own parser, keeping this layer as a thin pass-through. +func (i ConfigEnvsAddInput) validate() error { + if i.Value != nil && (i.SecretName != nil || i.ConfigMapName != nil) { + return fmt.Errorf("value is mutually exclusive with secretName/configMapName; provide one or the other") + } + if i.SecretKey != nil && i.SecretName == nil { + return fmt.Errorf("secretKey requires secretName to be set") + } + if i.ConfigMapKey != nil && i.ConfigMapName == nil { + return fmt.Errorf("configMapKey requires configMapName to be set") + } + if i.SecretName != nil && i.ConfigMapName != nil { + return fmt.Errorf("secretName and configMapName are mutually exclusive; provide only one source") + } + // All-keys import (no secretKey/configMapKey) is incompatible with --name: the + // underlying env validation only allows whole-secret/configMap templates when name is nil. + if i.SecretName != nil && i.SecretKey == nil && i.Name != nil { + return fmt.Errorf("name must not be set when importing all keys from a Secret (omit secretKey to import all keys)") + } + if i.ConfigMapName != nil && i.ConfigMapKey == nil && i.Name != nil { + return fmt.Errorf("name must not be set when importing all keys from a ConfigMap (omit configMapKey to import all keys)") + } + return nil } func (i ConfigEnvsAddInput) Args() []string { args := []string{"envs", "add", "--path", i.Path} args = appendStringFlag(args, "--name", i.Name) - args = appendStringFlag(args, "--value", i.Value) + + if i.Value != nil { + args = appendStringFlag(args, "--value", i.Value) + } else if i.SecretName != nil { + var v string + if i.SecretKey != nil { + v = fmt.Sprintf("{{ secret:%s:%s }}", *i.SecretName, *i.SecretKey) + } else { + v = fmt.Sprintf("{{ secret:%s }}", *i.SecretName) + } + args = append(args, "--value", v) + } else if i.ConfigMapName != nil { + var v string + if i.ConfigMapKey != nil { + v = fmt.Sprintf("{{ configMap:%s:%s }}", *i.ConfigMapName, *i.ConfigMapKey) + } else { + v = fmt.Sprintf("{{ configMap:%s }}", *i.ConfigMapName) + } + args = append(args, "--value", v) + } + args = appendBoolFlag(args, "--verbose", i.Verbose) return args } @@ -79,6 +142,9 @@ type ConfigEnvsAddOutput struct { } func (s *Server) configEnvsAddHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigEnvsAddInput) (result *mcp.CallToolResult, output ConfigEnvsAddOutput, err error) { + if err = input.validate(); err != nil { + return + } out, err := s.executor.Execute(ctx, "config", input.Args()...) if err != nil { err = fmt.Errorf("%w\n%s", err, string(out)) diff --git a/pkg/mcp/tools_config_envs_test.go b/pkg/mcp/tools_config_envs_test.go index 16440f701e..327ab06f9a 100644 --- a/pkg/mcp/tools_config_envs_test.go +++ b/pkg/mcp/tools_config_envs_test.go @@ -72,6 +72,390 @@ func TestTool_ConfigEnvsAdd(t *testing.T) { } } +// TestTool_ConfigEnvsAdd_SecretKey ensures secret-key-sourced env vars produce +// the correct "{{ secret:name:key }}" value template. +func TestTool_ConfigEnvsAdd_SecretKey(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + if subcommand != "config" { + t.Fatalf("expected subcommand 'config', got %q", subcommand) + } + if len(args) < 2 || args[0] != "envs" || args[1] != "add" { + t.Fatalf("expected positional args ['envs','add'], got %v", args[:min(2, len(args))]) + } + argsMap := argsToMap(args[2:]) + wantValue := "{{ secret:my-secret:MY_KEY }}" + if got, ok := argsMap["--value"]; !ok || got != wantValue { + t.Fatalf("expected --value=%q, got %q", wantValue, got) + } + if got, ok := argsMap["--name"]; !ok || got != "API_KEY" { + t.Fatalf("expected --name='API_KEY', got %q", got) + } + return []byte("Environment variable added successfully\n"), nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "config_envs_add", + Arguments: map[string]any{ + "path": ".", + "name": "API_KEY", + "secretName": "my-secret", + "secretKey": "MY_KEY", + }, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", result) + } + if !executor.ExecuteInvoked { + t.Fatal("executor was not invoked") + } +} + +// TestTool_ConfigEnvsAdd_SecretAllKeys ensures importing all keys from a Secret +// produces the "{{ secret:name }}" value template without --name. +func TestTool_ConfigEnvsAdd_SecretAllKeys(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + if subcommand != "config" { + t.Fatalf("expected subcommand 'config', got %q", subcommand) + } + if len(args) < 2 || args[0] != "envs" || args[1] != "add" { + t.Fatalf("expected positional args ['envs','add'], got %v", args[:min(2, len(args))]) + } + argsMap := argsToMap(args[2:]) + wantValue := "{{ secret:my-secret }}" + if got, ok := argsMap["--value"]; !ok || got != wantValue { + t.Fatalf("expected --value=%q, got %q", wantValue, got) + } + if _, ok := argsMap["--name"]; ok { + t.Fatal("expected no --name flag when importing all secret keys") + } + return []byte("Environment variable added successfully\n"), nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "config_envs_add", + Arguments: map[string]any{ + "path": ".", + "secretName": "my-secret", + }, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", result) + } + if !executor.ExecuteInvoked { + t.Fatal("executor was not invoked") + } +} + +// TestTool_ConfigEnvsAdd_ConfigMapKey ensures configmap-key-sourced env vars produce +// the correct "{{ configMap:name:key }}" value template. +func TestTool_ConfigEnvsAdd_ConfigMapKey(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + if subcommand != "config" { + t.Fatalf("expected subcommand 'config', got %q", subcommand) + } + if len(args) < 2 || args[0] != "envs" || args[1] != "add" { + t.Fatalf("expected positional args ['envs','add'], got %v", args[:min(2, len(args))]) + } + argsMap := argsToMap(args[2:]) + wantValue := "{{ configMap:my-config:DB_HOST }}" + if got, ok := argsMap["--value"]; !ok || got != wantValue { + t.Fatalf("expected --value=%q, got %q", wantValue, got) + } + if got, ok := argsMap["--name"]; !ok || got != "DATABASE_HOST" { + t.Fatalf("expected --name='DATABASE_HOST', got %q", got) + } + return []byte("Environment variable added successfully\n"), nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "config_envs_add", + Arguments: map[string]any{ + "path": ".", + "name": "DATABASE_HOST", + "configMapName": "my-config", + "configMapKey": "DB_HOST", + }, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", result) + } + if !executor.ExecuteInvoked { + t.Fatal("executor was not invoked") + } +} + +// TestTool_ConfigEnvsAdd_ConfigMapAllKeys ensures importing all keys from a ConfigMap +// produces the "{{ configMap:name }}" value template without --name. +func TestTool_ConfigEnvsAdd_ConfigMapAllKeys(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + if subcommand != "config" { + t.Fatalf("expected subcommand 'config', got %q", subcommand) + } + if len(args) < 2 || args[0] != "envs" || args[1] != "add" { + t.Fatalf("expected positional args ['envs','add'], got %v", args[:min(2, len(args))]) + } + argsMap := argsToMap(args[2:]) + wantValue := "{{ configMap:my-config }}" + if got, ok := argsMap["--value"]; !ok || got != wantValue { + t.Fatalf("expected --value=%q, got %q", wantValue, got) + } + if _, ok := argsMap["--name"]; ok { + t.Fatal("expected no --name flag when importing all configmap keys") + } + return []byte("Environment variable added successfully\n"), nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "config_envs_add", + Arguments: map[string]any{ + "path": ".", + "configMapName": "my-config", + }, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("unexpected error result: %v", result) + } + if !executor.ExecuteInvoked { + t.Fatal("executor was not invoked") + } +} + +// TestTool_ConfigEnvsAdd_ValueAndSecretMutuallyExclusive ensures a validation +// error is returned when both Value and SecretName are provided together. +func TestTool_ConfigEnvsAdd_ValueAndSecretMutuallyExclusive(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + t.Fatal("executor must not be called when input is invalid") + return nil, nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "config_envs_add", + Arguments: map[string]any{ + "path": ".", + "name": "MY_VAR", + "value": "explicit-value", + "secretName": "my-secret", + }, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result when value and secretName are both provided") + } + if executor.ExecuteInvoked { + t.Fatal("executor must not be invoked when input fails validation") + } +} + +// TestTool_ConfigEnvsAdd_NameWithSecretAllKeys ensures a validation error is returned +// when name is provided alongside an all-keys Secret import (no secretKey). +func TestTool_ConfigEnvsAdd_NameWithSecretAllKeys(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + t.Fatal("executor must not be called when input is invalid") + return nil, nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "config_envs_add", + Arguments: map[string]any{ + "path": ".", + "name": "MY_VAR", + "secretName": "my-secret", + }, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result when name is set alongside an all-keys Secret import") + } + if executor.ExecuteInvoked { + t.Fatal("executor must not be invoked when input fails validation") + } +} + +// TestTool_ConfigEnvsAdd_NameWithConfigMapAllKeys ensures a validation error is returned +// when name is provided alongside an all-keys ConfigMap import (no configMapKey). +func TestTool_ConfigEnvsAdd_NameWithConfigMapAllKeys(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + t.Fatal("executor must not be called when input is invalid") + return nil, nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "config_envs_add", + Arguments: map[string]any{ + "path": ".", + "name": "MY_VAR", + "configMapName": "my-config", + }, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result when name is set alongside an all-keys ConfigMap import") + } + if executor.ExecuteInvoked { + t.Fatal("executor must not be invoked when input fails validation") + } +} + +// TestTool_ConfigEnvsAdd_BothSecretAndConfigMap ensures a validation error is returned +// when both secretName and configMapName are provided simultaneously. +func TestTool_ConfigEnvsAdd_BothSecretAndConfigMap(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + t.Fatal("executor must not be called when input is invalid") + return nil, nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "config_envs_add", + Arguments: map[string]any{ + "path": ".", + "name": "MY_VAR", + "secretName": "my-secret", + "configMapName": "my-config", + }, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result when both secretName and configMapName are provided") + } + if executor.ExecuteInvoked { + t.Fatal("executor must not be invoked when input fails validation") + } +} + +// TestTool_ConfigEnvsAdd_SecretKeyWithoutSecretName ensures a validation error is returned +// when secretKey is provided without secretName (SEVERE-2). +func TestTool_ConfigEnvsAdd_SecretKeyWithoutSecretName(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + t.Fatal("executor must not be called when input is invalid") + return nil, nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "config_envs_add", + Arguments: map[string]any{ + "path": ".", + "name": "MY_VAR", + "secretKey": "MY_KEY", + }, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result when secretKey is set without secretName") + } + if executor.ExecuteInvoked { + t.Fatal("executor must not be invoked when input fails validation") + } +} + +// TestTool_ConfigEnvsAdd_ConfigMapKeyWithoutConfigMapName ensures a validation error is +// returned when configMapKey is provided without configMapName (SEVERE-2). +func TestTool_ConfigEnvsAdd_ConfigMapKeyWithoutConfigMapName(t *testing.T) { + executor := mock.NewExecutor() + executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { + t.Fatal("executor must not be called when input is invalid") + return nil, nil + } + + client, _, err := newTestPair(t, WithExecutor(executor)) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "config_envs_add", + Arguments: map[string]any{ + "path": ".", + "name": "MY_VAR", + "configMapKey": "MY_KEY", + }, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result when configMapKey is set without configMapName") + } + if executor.ExecuteInvoked { + t.Fatal("executor must not be invoked when input fails validation") + } +} + // TestTool_ConfigEnvsList ensures the config_envs_list tool lists environment variables. func TestTool_ConfigEnvsList(t *testing.T) { executor := mock.NewExecutor()