From e1d49c15e0b7454701f716251ca004ad37d5dcb9 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Wed, 25 Feb 2026 15:45:02 +0000 Subject: [PATCH 1/4] Support secret references for MCP server headers and env Add headersFrom and envFrom fields to MCPServerSpec that reference Kubernetes Secrets. The controller resolves these secrets in createJob before building the MCP servers JSON, merging secret values into inline headers/env maps with secret values taking precedence for overlapping keys. Fixes #313 Co-Authored-By: Claude Opus 4.6 --- api/v1alpha1/agentconfig_types.go | 14 ++ api/v1alpha1/zz_generated.deepcopy.go | 10 + install-crd.yaml | 26 +++ internal/controller/job_builder_test.go | 149 +++++++++++++++ internal/controller/task_controller.go | 73 ++++++++ internal/controller/task_controller_test.go | 195 +++++++++++++++++++- internal/manifests/install-crd.yaml | 26 +++ 7 files changed, 491 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/agentconfig_types.go b/api/v1alpha1/agentconfig_types.go index 13d34be5..3d373ed7 100644 --- a/api/v1alpha1/agentconfig_types.go +++ b/api/v1alpha1/agentconfig_types.go @@ -88,10 +88,24 @@ type MCPServerSpec struct { // +optional Headers map[string]string `json:"headers,omitempty"` + // HeadersFrom references a Secret whose data keys are header names + // and values are header values. Only used when type is "http" or "sse". + // Values from HeadersFrom take precedence over inline Headers for + // overlapping keys. + // +optional + HeadersFrom *SecretReference `json:"headersFrom,omitempty"` + // Env are environment variables for the server process. // Only used when type is "stdio". // +optional Env map[string]string `json:"env,omitempty"` + + // EnvFrom references a Secret whose data keys are environment variable + // names and values are environment variable values. Only used when + // type is "stdio". Values from EnvFrom take precedence over inline Env + // for overlapping keys. + // +optional + EnvFrom *SecretReference `json:"envFrom,omitempty"` } // AgentConfigReference refers to an AgentConfig resource by name. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 954d0f8a..ecc4e9a1 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -250,6 +250,11 @@ func (in *MCPServerSpec) DeepCopyInto(out *MCPServerSpec) { (*out)[key] = val } } + if in.HeadersFrom != nil { + in, out := &in.HeadersFrom, &out.HeadersFrom + *out = new(SecretReference) + **out = **in + } if in.Env != nil { in, out := &in.Env, &out.Env *out = make(map[string]string, len(*in)) @@ -257,6 +262,11 @@ func (in *MCPServerSpec) DeepCopyInto(out *MCPServerSpec) { (*out)[key] = val } } + if in.EnvFrom != nil { + in, out := &in.EnvFrom, &out.EnvFrom + *out = new(SecretReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPServerSpec. diff --git a/install-crd.yaml b/install-crd.yaml index a219b46a..5f1e29dc 100644 --- a/install-crd.yaml +++ b/install-crd.yaml @@ -72,6 +72,19 @@ spec: Env are environment variables for the server process. Only used when type is "stdio". type: object + envFrom: + description: |- + EnvFrom references a Secret whose data keys are environment variable + names and values are environment variable values. Only used when + type is "stdio". Values from EnvFrom take precedence over inline Env + for overlapping keys. + properties: + name: + description: Name is the name of the secret. + type: string + required: + - name + type: object headers: additionalProperties: type: string @@ -79,6 +92,19 @@ spec: Headers are HTTP headers to include in requests. Only used when type is "http" or "sse". type: object + headersFrom: + description: |- + HeadersFrom references a Secret whose data keys are header names + and values are header values. Only used when type is "http" or "sse". + Values from HeadersFrom take precedence over inline Headers for + overlapping keys. + properties: + name: + description: Name is the name of the secret. + type: string + required: + - name + type: object name: description: |- Name identifies this MCP server. Used as the key in the diff --git a/internal/controller/job_builder_test.go b/internal/controller/job_builder_test.go index e1a32738..7a639e79 100644 --- a/internal/controller/job_builder_test.go +++ b/internal/controller/job_builder_test.go @@ -3482,3 +3482,152 @@ func TestBuildJob_WorkspaceWithInvalidUpstreamRemoteNoEnv(t *testing.T) { } } } + +// TestBuildJob_AgentConfigMCPServersResolvedHeadersFrom verifies that headers +// resolved from a secret (merged into the inline Headers map by the controller) +// appear correctly in the AXON_MCP_SERVERS JSON output. +func TestBuildJob_AgentConfigMCPServersResolvedHeadersFrom(t *testing.T) { + builder := NewJobBuilder() + task := &axonv1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-mcp-resolved", + Namespace: "default", + }, + Spec: axonv1alpha1.TaskSpec{ + Type: AgentTypeClaudeCode, + Prompt: "Fix issue", + Credentials: axonv1alpha1.Credentials{ + Type: axonv1alpha1.CredentialTypeAPIKey, + SecretRef: axonv1alpha1.SecretReference{Name: "my-secret"}, + }, + }, + } + + // Simulate what the controller does: secret headers are already merged + // into the inline Headers map, and HeadersFrom is cleared. + agentConfig := &axonv1alpha1.AgentConfigSpec{ + MCPServers: []axonv1alpha1.MCPServerSpec{ + { + Name: "github", + Type: "http", + URL: "https://api.githubcopilot.com/mcp/", + Headers: map[string]string{ + "Authorization": "Bearer secret-token", + "X-Custom": "custom-value", + }, + // HeadersFrom is nil after controller resolution + }, + }, + } + + job, err := builder.Build(task, nil, agentConfig, task.Spec.Prompt) + if err != nil { + t.Fatalf("Build() returned error: %v", err) + } + + container := job.Spec.Template.Spec.Containers[0] + + var mcpJSON string + for _, env := range container.Env { + if env.Name == "AXON_MCP_SERVERS" { + mcpJSON = env.Value + } + } + if mcpJSON == "" { + t.Fatal("Expected AXON_MCP_SERVERS env var to be set") + } + + var parsed struct { + MCPServers map[string]struct { + Headers map[string]string `json:"headers"` + } `json:"mcpServers"` + } + if err := json.Unmarshal([]byte(mcpJSON), &parsed); err != nil { + t.Fatalf("Failed to parse AXON_MCP_SERVERS JSON: %v", err) + } + + github, ok := parsed.MCPServers["github"] + if !ok { + t.Fatal("Expected 'github' MCP server entry") + } + if github.Headers["Authorization"] != "Bearer secret-token" { + t.Errorf("Expected Authorization header from resolved secret, got %q", github.Headers["Authorization"]) + } + if github.Headers["X-Custom"] != "custom-value" { + t.Errorf("Expected X-Custom header from resolved secret, got %q", github.Headers["X-Custom"]) + } +} + +// TestBuildJob_AgentConfigMCPServersResolvedEnvFrom verifies that env vars +// resolved from a secret appear correctly in the AXON_MCP_SERVERS JSON output. +func TestBuildJob_AgentConfigMCPServersResolvedEnvFrom(t *testing.T) { + builder := NewJobBuilder() + task := &axonv1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-mcp-env-resolved", + Namespace: "default", + }, + Spec: axonv1alpha1.TaskSpec{ + Type: AgentTypeClaudeCode, + Prompt: "Fix issue", + Credentials: axonv1alpha1.Credentials{ + Type: axonv1alpha1.CredentialTypeAPIKey, + SecretRef: axonv1alpha1.SecretReference{Name: "my-secret"}, + }, + }, + } + + // Simulate controller resolution: secret env values merged into Env map + agentConfig := &axonv1alpha1.AgentConfigSpec{ + MCPServers: []axonv1alpha1.MCPServerSpec{ + { + Name: "local-db", + Type: "stdio", + Command: "npx", + Args: []string{"-y", "dbhub"}, + Env: map[string]string{ + "DSN": "postgres://localhost/db", + "DB_PASSWORD": "secret-pass", + }, + // EnvFrom is nil after controller resolution + }, + }, + } + + job, err := builder.Build(task, nil, agentConfig, task.Spec.Prompt) + if err != nil { + t.Fatalf("Build() returned error: %v", err) + } + + container := job.Spec.Template.Spec.Containers[0] + + var mcpJSON string + for _, env := range container.Env { + if env.Name == "AXON_MCP_SERVERS" { + mcpJSON = env.Value + } + } + if mcpJSON == "" { + t.Fatal("Expected AXON_MCP_SERVERS env var to be set") + } + + var parsed struct { + MCPServers map[string]struct { + Env map[string]string `json:"env"` + } `json:"mcpServers"` + } + if err := json.Unmarshal([]byte(mcpJSON), &parsed); err != nil { + t.Fatalf("Failed to parse AXON_MCP_SERVERS JSON: %v", err) + } + + localDB, ok := parsed.MCPServers["local-db"] + if !ok { + t.Fatal("Expected 'local-db' MCP server entry") + } + if localDB.Env["DSN"] != "postgres://localhost/db" { + t.Errorf("Expected DSN env, got %q", localDB.Env["DSN"]) + } + if localDB.Env["DB_PASSWORD"] != "secret-pass" { + t.Errorf("Expected DB_PASSWORD env from resolved secret, got %q", localDB.Env["DB_PASSWORD"]) + } +} diff --git a/internal/controller/task_controller.go b/internal/controller/task_controller.go index 1206b385..3c3ac475 100644 --- a/internal/controller/task_controller.go +++ b/internal/controller/task_controller.go @@ -257,6 +257,28 @@ func (r *TaskReconciler) createJob(ctx context.Context, task *axonv1alpha1.Task) return ctrl.Result{}, err } agentConfig = &ac.Spec + + // Resolve MCP server secret references (headersFrom / envFrom) + if len(agentConfig.MCPServers) > 0 { + resolved, err := r.resolveMCPServerSecrets(ctx, task.Namespace, agentConfig.MCPServers) + if err != nil { + logger.Error(err, "Unable to resolve MCP server secrets") + r.recordEvent(task, corev1.EventTypeWarning, "MCPSecretFailed", "Failed to resolve MCP server secret: %v", err) + updateErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + if getErr := r.Get(ctx, client.ObjectKeyFromObject(task), task); getErr != nil { + return getErr + } + task.Status.Phase = axonv1alpha1.TaskPhaseFailed + task.Status.Message = fmt.Sprintf("Failed to resolve MCP server secret: %v", err) + return r.Status().Update(ctx, task) + }) + if updateErr != nil { + logger.Error(updateErr, "Unable to update Task status") + } + return ctrl.Result{}, nil + } + agentConfig.MCPServers = resolved + } } resolvedPrompt := r.resolvePromptTemplate(ctx, task) @@ -387,6 +409,57 @@ func (r *TaskReconciler) resolveGitHubAppToken(ctx context.Context, task *axonv1 return &resolved, nil } +// resolveMCPServerSecrets resolves headersFrom and envFrom secret references +// for MCP server specs. It fetches the referenced secrets and merges their data +// into the inline Headers/Env maps (with secret values taking precedence for +// overlapping keys). The returned slice contains copies with the merged values +// and nil headersFrom/envFrom fields. +func (r *TaskReconciler) resolveMCPServerSecrets(ctx context.Context, namespace string, servers []axonv1alpha1.MCPServerSpec) ([]axonv1alpha1.MCPServerSpec, error) { + resolved := make([]axonv1alpha1.MCPServerSpec, len(servers)) + for i, s := range servers { + resolved[i] = s + + if s.HeadersFrom != nil { + var secret corev1.Secret + if err := r.Get(ctx, client.ObjectKey{ + Namespace: namespace, + Name: s.HeadersFrom.Name, + }, &secret); err != nil { + return nil, fmt.Errorf("fetching headersFrom secret %q for MCP server %q: %w", s.HeadersFrom.Name, s.Name, err) + } + merged := make(map[string]string, len(s.Headers)+len(secret.Data)) + for k, v := range s.Headers { + merged[k] = v + } + for k, v := range secret.Data { + merged[k] = string(v) + } + resolved[i].Headers = merged + resolved[i].HeadersFrom = nil + } + + if s.EnvFrom != nil { + var secret corev1.Secret + if err := r.Get(ctx, client.ObjectKey{ + Namespace: namespace, + Name: s.EnvFrom.Name, + }, &secret); err != nil { + return nil, fmt.Errorf("fetching envFrom secret %q for MCP server %q: %w", s.EnvFrom.Name, s.Name, err) + } + merged := make(map[string]string, len(s.Env)+len(secret.Data)) + for k, v := range s.Env { + merged[k] = v + } + for k, v := range secret.Data { + merged[k] = string(v) + } + resolved[i].Env = merged + resolved[i].EnvFrom = nil + } + } + return resolved, nil +} + // updateStatus updates Task status based on Job status. func (r *TaskReconciler) updateStatus(ctx context.Context, task *axonv1alpha1.Task, job *batchv1.Job) (ctrl.Result, error) { logger := log.FromContext(ctx) diff --git a/internal/controller/task_controller_test.go b/internal/controller/task_controller_test.go index db35e386..62a6e51b 100644 --- a/internal/controller/task_controller_test.go +++ b/internal/controller/task_controller_test.go @@ -1,12 +1,17 @@ package controller import ( + "context" "testing" "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - axonv1alpha1 "github.com/axon-core/axon/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func TestTTLExpired(t *testing.T) { @@ -158,3 +163,189 @@ func TestTTLExpired(t *testing.T) { }) } } + +func newReconcilerWithFakeClient(objs ...runtime.Object) *TaskReconciler { + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(axonv1alpha1.AddToScheme(scheme)) + + clientObjs := make([]runtime.Object, len(objs)) + copy(clientObjs, objs) + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(clientObjs...). + Build() + return &TaskReconciler{ + Client: cl, + Scheme: scheme, + } +} + +func TestResolveMCPServerSecrets_HeadersFrom(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-headers", + Namespace: "default", + }, + Data: map[string][]byte{ + "Authorization": []byte("Bearer secret-token"), + "X-Custom": []byte("custom-value"), + }, + } + + r := newReconcilerWithFakeClient(secret) + + servers := []axonv1alpha1.MCPServerSpec{ + { + Name: "github", + Type: "http", + URL: "https://api.example.com/mcp/", + Headers: map[string]string{ + "X-Static": "static-value", + }, + HeadersFrom: &axonv1alpha1.SecretReference{Name: "mcp-headers"}, + }, + } + + resolved, err := r.resolveMCPServerSecrets(context.Background(), "default", servers) + if err != nil { + t.Fatalf("resolveMCPServerSecrets() returned error: %v", err) + } + + if len(resolved) != 1 { + t.Fatalf("Expected 1 resolved server, got %d", len(resolved)) + } + + s := resolved[0] + if s.HeadersFrom != nil { + t.Error("Expected HeadersFrom to be nil after resolution") + } + if s.Headers["Authorization"] != "Bearer secret-token" { + t.Errorf("Expected Authorization header from secret, got %q", s.Headers["Authorization"]) + } + if s.Headers["X-Custom"] != "custom-value" { + t.Errorf("Expected X-Custom header from secret, got %q", s.Headers["X-Custom"]) + } + if s.Headers["X-Static"] != "static-value" { + t.Errorf("Expected X-Static inline header preserved, got %q", s.Headers["X-Static"]) + } +} + +func TestResolveMCPServerSecrets_HeadersFromPrecedence(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-headers", + Namespace: "default", + }, + Data: map[string][]byte{ + "Authorization": []byte("Bearer from-secret"), + }, + } + + r := newReconcilerWithFakeClient(secret) + + servers := []axonv1alpha1.MCPServerSpec{ + { + Name: "api", + Type: "http", + URL: "https://api.example.com/mcp/", + Headers: map[string]string{ + "Authorization": "Bearer inline-token", + }, + HeadersFrom: &axonv1alpha1.SecretReference{Name: "mcp-headers"}, + }, + } + + resolved, err := r.resolveMCPServerSecrets(context.Background(), "default", servers) + if err != nil { + t.Fatalf("resolveMCPServerSecrets() returned error: %v", err) + } + + // Secret value should take precedence over inline value + if resolved[0].Headers["Authorization"] != "Bearer from-secret" { + t.Errorf("Expected headersFrom to take precedence, got %q", resolved[0].Headers["Authorization"]) + } +} + +func TestResolveMCPServerSecrets_EnvFrom(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-env", + Namespace: "default", + }, + Data: map[string][]byte{ + "DB_PASSWORD": []byte("secret-pass"), + }, + } + + r := newReconcilerWithFakeClient(secret) + + servers := []axonv1alpha1.MCPServerSpec{ + { + Name: "local-db", + Type: "stdio", + Command: "npx", + Args: []string{"-y", "dbhub"}, + Env: map[string]string{ + "DSN": "postgres://localhost/db", + }, + EnvFrom: &axonv1alpha1.SecretReference{Name: "mcp-env"}, + }, + } + + resolved, err := r.resolveMCPServerSecrets(context.Background(), "default", servers) + if err != nil { + t.Fatalf("resolveMCPServerSecrets() returned error: %v", err) + } + + s := resolved[0] + if s.EnvFrom != nil { + t.Error("Expected EnvFrom to be nil after resolution") + } + if s.Env["DB_PASSWORD"] != "secret-pass" { + t.Errorf("Expected DB_PASSWORD from secret, got %q", s.Env["DB_PASSWORD"]) + } + if s.Env["DSN"] != "postgres://localhost/db" { + t.Errorf("Expected DSN inline env preserved, got %q", s.Env["DSN"]) + } +} + +func TestResolveMCPServerSecrets_MissingSecret(t *testing.T) { + r := newReconcilerWithFakeClient() // No secrets in the cluster + + servers := []axonv1alpha1.MCPServerSpec{ + { + Name: "api", + Type: "http", + URL: "https://api.example.com/mcp/", + HeadersFrom: &axonv1alpha1.SecretReference{Name: "nonexistent"}, + }, + } + + _, err := r.resolveMCPServerSecrets(context.Background(), "default", servers) + if err == nil { + t.Fatal("Expected error for missing secret, got nil") + } +} + +func TestResolveMCPServerSecrets_NoSecretRefs(t *testing.T) { + r := newReconcilerWithFakeClient() + + servers := []axonv1alpha1.MCPServerSpec{ + { + Name: "github", + Type: "http", + URL: "https://api.example.com/mcp/", + Headers: map[string]string{"X-Static": "value"}, + }, + } + + resolved, err := r.resolveMCPServerSecrets(context.Background(), "default", servers) + if err != nil { + t.Fatalf("resolveMCPServerSecrets() returned error: %v", err) + } + + if resolved[0].Headers["X-Static"] != "value" { + t.Errorf("Expected inline headers preserved, got %v", resolved[0].Headers) + } +} diff --git a/internal/manifests/install-crd.yaml b/internal/manifests/install-crd.yaml index a219b46a..5f1e29dc 100644 --- a/internal/manifests/install-crd.yaml +++ b/internal/manifests/install-crd.yaml @@ -72,6 +72,19 @@ spec: Env are environment variables for the server process. Only used when type is "stdio". type: object + envFrom: + description: |- + EnvFrom references a Secret whose data keys are environment variable + names and values are environment variable values. Only used when + type is "stdio". Values from EnvFrom take precedence over inline Env + for overlapping keys. + properties: + name: + description: Name is the name of the secret. + type: string + required: + - name + type: object headers: additionalProperties: type: string @@ -79,6 +92,19 @@ spec: Headers are HTTP headers to include in requests. Only used when type is "http" or "sse". type: object + headersFrom: + description: |- + HeadersFrom references a Secret whose data keys are header names + and values are header values. Only used when type is "http" or "sse". + Values from HeadersFrom take precedence over inline Headers for + overlapping keys. + properties: + name: + description: Name is the name of the secret. + type: string + required: + - name + type: object name: description: |- Name identifies this MCP server. Used as the key in the From d5853ede2e7cd57ed0e86f17554d2b02404d529e Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Wed, 25 Feb 2026 16:12:26 +0000 Subject: [PATCH 2/4] Add secretRef nesting to headersFrom/envFrom fields Wrap SecretReference in a SecretValuesSource struct so that headersFrom and envFrom use a nested secretRef field. This follows the same pattern as Credentials.SecretRef and makes the API extensible for future non-secret sources. Co-Authored-By: Claude Opus 4.6 --- api/v1alpha1/agentconfig_types.go | 10 +++++-- api/v1alpha1/zz_generated.deepcopy.go | 20 ++++++++++++-- install-crd.yaml | 30 +++++++++++++++------ internal/controller/task_controller.go | 8 +++--- internal/controller/task_controller_test.go | 8 +++--- internal/manifests/install-crd.yaml | 30 +++++++++++++++------ 6 files changed, 78 insertions(+), 28 deletions(-) diff --git a/api/v1alpha1/agentconfig_types.go b/api/v1alpha1/agentconfig_types.go index 3d373ed7..27a7e480 100644 --- a/api/v1alpha1/agentconfig_types.go +++ b/api/v1alpha1/agentconfig_types.go @@ -93,7 +93,7 @@ type MCPServerSpec struct { // Values from HeadersFrom take precedence over inline Headers for // overlapping keys. // +optional - HeadersFrom *SecretReference `json:"headersFrom,omitempty"` + HeadersFrom *SecretValuesSource `json:"headersFrom,omitempty"` // Env are environment variables for the server process. // Only used when type is "stdio". @@ -105,7 +105,13 @@ type MCPServerSpec struct { // type is "stdio". Values from EnvFrom take precedence over inline Env // for overlapping keys. // +optional - EnvFrom *SecretReference `json:"envFrom,omitempty"` + EnvFrom *SecretValuesSource `json:"envFrom,omitempty"` +} + +// SecretValuesSource selects a Secret to populate values from. +type SecretValuesSource struct { + // SecretRef references the Secret to read data from. + SecretRef SecretReference `json:"secretRef"` } // AgentConfigReference refers to an AgentConfig resource by name. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index ecc4e9a1..97f87819 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -252,7 +252,7 @@ func (in *MCPServerSpec) DeepCopyInto(out *MCPServerSpec) { } if in.HeadersFrom != nil { in, out := &in.HeadersFrom, &out.HeadersFrom - *out = new(SecretReference) + *out = new(SecretValuesSource) **out = **in } if in.Env != nil { @@ -264,7 +264,7 @@ func (in *MCPServerSpec) DeepCopyInto(out *MCPServerSpec) { } if in.EnvFrom != nil { in, out := &in.EnvFrom, &out.EnvFrom - *out = new(SecretReference) + *out = new(SecretValuesSource) **out = **in } } @@ -358,6 +358,22 @@ func (in *SecretReference) DeepCopy() *SecretReference { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretValuesSource) DeepCopyInto(out *SecretValuesSource) { + *out = *in + out.SecretRef = in.SecretRef +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretValuesSource. +func (in *SecretValuesSource) DeepCopy() *SecretValuesSource { + if in == nil { + return nil + } + out := new(SecretValuesSource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SkillDefinition) DeepCopyInto(out *SkillDefinition) { *out = *in diff --git a/install-crd.yaml b/install-crd.yaml index 5f1e29dc..9a64beff 100644 --- a/install-crd.yaml +++ b/install-crd.yaml @@ -79,11 +79,18 @@ spec: type is "stdio". Values from EnvFrom take precedence over inline Env for overlapping keys. properties: - name: - description: Name is the name of the secret. - type: string + secretRef: + description: SecretRef references the Secret to read data + from. + properties: + name: + description: Name is the name of the secret. + type: string + required: + - name + type: object required: - - name + - secretRef type: object headers: additionalProperties: @@ -99,11 +106,18 @@ spec: Values from HeadersFrom take precedence over inline Headers for overlapping keys. properties: - name: - description: Name is the name of the secret. - type: string + secretRef: + description: SecretRef references the Secret to read data + from. + properties: + name: + description: Name is the name of the secret. + type: string + required: + - name + type: object required: - - name + - secretRef type: object name: description: |- diff --git a/internal/controller/task_controller.go b/internal/controller/task_controller.go index 3c3ac475..272f3d47 100644 --- a/internal/controller/task_controller.go +++ b/internal/controller/task_controller.go @@ -423,9 +423,9 @@ func (r *TaskReconciler) resolveMCPServerSecrets(ctx context.Context, namespace var secret corev1.Secret if err := r.Get(ctx, client.ObjectKey{ Namespace: namespace, - Name: s.HeadersFrom.Name, + Name: s.HeadersFrom.SecretRef.Name, }, &secret); err != nil { - return nil, fmt.Errorf("fetching headersFrom secret %q for MCP server %q: %w", s.HeadersFrom.Name, s.Name, err) + return nil, fmt.Errorf("fetching headersFrom secret %q for MCP server %q: %w", s.HeadersFrom.SecretRef.Name, s.Name, err) } merged := make(map[string]string, len(s.Headers)+len(secret.Data)) for k, v := range s.Headers { @@ -442,9 +442,9 @@ func (r *TaskReconciler) resolveMCPServerSecrets(ctx context.Context, namespace var secret corev1.Secret if err := r.Get(ctx, client.ObjectKey{ Namespace: namespace, - Name: s.EnvFrom.Name, + Name: s.EnvFrom.SecretRef.Name, }, &secret); err != nil { - return nil, fmt.Errorf("fetching envFrom secret %q for MCP server %q: %w", s.EnvFrom.Name, s.Name, err) + return nil, fmt.Errorf("fetching envFrom secret %q for MCP server %q: %w", s.EnvFrom.SecretRef.Name, s.Name, err) } merged := make(map[string]string, len(s.Env)+len(secret.Data)) for k, v := range s.Env { diff --git a/internal/controller/task_controller_test.go b/internal/controller/task_controller_test.go index 62a6e51b..579c3639 100644 --- a/internal/controller/task_controller_test.go +++ b/internal/controller/task_controller_test.go @@ -203,7 +203,7 @@ func TestResolveMCPServerSecrets_HeadersFrom(t *testing.T) { Headers: map[string]string{ "X-Static": "static-value", }, - HeadersFrom: &axonv1alpha1.SecretReference{Name: "mcp-headers"}, + HeadersFrom: &axonv1alpha1.SecretValuesSource{SecretRef: axonv1alpha1.SecretReference{Name: "mcp-headers"}}, }, } @@ -252,7 +252,7 @@ func TestResolveMCPServerSecrets_HeadersFromPrecedence(t *testing.T) { Headers: map[string]string{ "Authorization": "Bearer inline-token", }, - HeadersFrom: &axonv1alpha1.SecretReference{Name: "mcp-headers"}, + HeadersFrom: &axonv1alpha1.SecretValuesSource{SecretRef: axonv1alpha1.SecretReference{Name: "mcp-headers"}}, }, } @@ -289,7 +289,7 @@ func TestResolveMCPServerSecrets_EnvFrom(t *testing.T) { Env: map[string]string{ "DSN": "postgres://localhost/db", }, - EnvFrom: &axonv1alpha1.SecretReference{Name: "mcp-env"}, + EnvFrom: &axonv1alpha1.SecretValuesSource{SecretRef: axonv1alpha1.SecretReference{Name: "mcp-env"}}, }, } @@ -318,7 +318,7 @@ func TestResolveMCPServerSecrets_MissingSecret(t *testing.T) { Name: "api", Type: "http", URL: "https://api.example.com/mcp/", - HeadersFrom: &axonv1alpha1.SecretReference{Name: "nonexistent"}, + HeadersFrom: &axonv1alpha1.SecretValuesSource{SecretRef: axonv1alpha1.SecretReference{Name: "nonexistent"}}, }, } diff --git a/internal/manifests/install-crd.yaml b/internal/manifests/install-crd.yaml index 5f1e29dc..9a64beff 100644 --- a/internal/manifests/install-crd.yaml +++ b/internal/manifests/install-crd.yaml @@ -79,11 +79,18 @@ spec: type is "stdio". Values from EnvFrom take precedence over inline Env for overlapping keys. properties: - name: - description: Name is the name of the secret. - type: string + secretRef: + description: SecretRef references the Secret to read data + from. + properties: + name: + description: Name is the name of the secret. + type: string + required: + - name + type: object required: - - name + - secretRef type: object headers: additionalProperties: @@ -99,11 +106,18 @@ spec: Values from HeadersFrom take precedence over inline Headers for overlapping keys. properties: - name: - description: Name is the name of the secret. - type: string + secretRef: + description: SecretRef references the Secret to read data + from. + properties: + name: + description: Name is the name of the secret. + type: string + required: + - name + type: object required: - - name + - secretRef type: object name: description: |- From c10431b397f4422b9292e76893ee64a63cc6d40f Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Wed, 25 Feb 2026 15:58:21 +0000 Subject: [PATCH 3/4] Add comment-based workflow control for GitHub issues Implements comment-based workflow control from #417 to support autonomous workflows on repositories where label permissions are unavailable. - Adds triggerComment field to GitHubIssues spec: include issues only when a comment matches (e.g., /axon pick-up). Also acts as resume command when used with excludeComments. - Adds excludeComments field ([]string): exclude issues with a comment matching any of these strings. Supports multiple exclusion patterns, consistent with excludeLabels. - Comments scanned in reverse chronological order; most recent wins. - Commands must appear on their own line to avoid false matches in prose. - Labels remain the default; comment-based control is a fallback. Files changed: - api/v1alpha1/taskspawner_types.go: new comment fields on GitHubIssues - api/v1alpha1/zz_generated.deepcopy.go: auto-generated - cmd/axon-spawner/main.go: comment field passthrough to GitHubSource - internal/source/github.go: passesCommentFilter, containsCommand, containsAnyCommand logic - install-crd.yaml, internal/manifests/install-crd.yaml: CRD updates - Test files with comprehensive unit and integration coverage Closes #417 Co-Authored-By: Claude Opus 4.6 --- api/v1alpha1/taskspawner_types.go | 18 ++ api/v1alpha1/zz_generated.deepcopy.go | 5 + cmd/axon-spawner/main.go | 18 +- cmd/axon-spawner/main_test.go | 24 ++ install-crd.yaml | 20 ++ internal/manifests/install-crd.yaml | 20 ++ internal/source/github.go | 101 +++++++- internal/source/github_test.go | 347 ++++++++++++++++++++++++++ test/integration/taskspawner_test.go | 220 ++++++++++++++++ 9 files changed, 756 insertions(+), 17 deletions(-) diff --git a/api/v1alpha1/taskspawner_types.go b/api/v1alpha1/taskspawner_types.go index 11e12547..e1c6913c 100644 --- a/api/v1alpha1/taskspawner_types.go +++ b/api/v1alpha1/taskspawner_types.go @@ -75,6 +75,24 @@ type GitHubIssues struct { // +kubebuilder:default=open // +optional State string `json:"state,omitempty"` + + // TriggerComment enables comment-based discovery. When set, only issues + // that have a comment matching this string (e.g., "/axon pick-up") are + // included. This is useful for repos where you lack label permissions. + // If ExcludeComments is also set, TriggerComment doubles as a resume + // command — the most recent match between TriggerComment and + // ExcludeComments wins. Comments are scanned in reverse chronological + // order. + // +optional + TriggerComment string `json:"triggerComment,omitempty"` + + // ExcludeComments enables comment-based exclusion. When set, issues that + // have a comment matching any of these strings (e.g., "/axon needs-input") + // are excluded unless a subsequent TriggerComment overrides it. Comments + // are scanned in reverse chronological order — the most recent matching + // command wins. + // +optional + ExcludeComments []string `json:"excludeComments,omitempty"` } // Jira discovers issues from a Jira project. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 97f87819..ed0ab09e 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -192,6 +192,11 @@ func (in *GitHubIssues) DeepCopyInto(out *GitHubIssues) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.ExcludeComments != nil { + in, out := &in.ExcludeComments, &out.ExcludeComments + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitHubIssues. diff --git a/cmd/axon-spawner/main.go b/cmd/axon-spawner/main.go index 02243c3e..ac2fc405 100644 --- a/cmd/axon-spawner/main.go +++ b/cmd/axon-spawner/main.go @@ -342,14 +342,16 @@ func buildSource(ts *axonv1alpha1.TaskSpawner, owner, repo, apiBaseURL, tokenFil } return &source.GitHubSource{ - Owner: owner, - Repo: repo, - Types: gh.Types, - Labels: gh.Labels, - ExcludeLabels: gh.ExcludeLabels, - State: gh.State, - Token: token, - BaseURL: apiBaseURL, + Owner: owner, + Repo: repo, + Types: gh.Types, + Labels: gh.Labels, + ExcludeLabels: gh.ExcludeLabels, + State: gh.State, + Token: token, + BaseURL: apiBaseURL, + TriggerComment: gh.TriggerComment, + ExcludeComments: gh.ExcludeComments, }, nil } diff --git a/cmd/axon-spawner/main_test.go b/cmd/axon-spawner/main_test.go index b1b8251d..a9b7f537 100644 --- a/cmd/axon-spawner/main_test.go +++ b/cmd/axon-spawner/main_test.go @@ -859,3 +859,27 @@ func TestRunCycleWithSource_NotSuspendedConditionCleared(t *testing.T) { } } } + +func TestRunCycleWithSource_CommentFieldsPassedToSource(t *testing.T) { + ts := newTaskSpawner("spawner", "default", nil) + ts.Spec.When.GitHubIssues = &axonv1alpha1.GitHubIssues{ + TriggerComment: "/axon pick-up", + ExcludeComments: []string{"/axon needs-input"}, + } + + src, err := buildSource(ts, "owner", "repo", "", "", "", "", "") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + ghSrc, ok := src.(*source.GitHubSource) + if !ok { + t.Fatalf("Expected *source.GitHubSource, got %T", src) + } + if ghSrc.TriggerComment != "/axon pick-up" { + t.Errorf("TriggerComment = %q, want %q", ghSrc.TriggerComment, "/axon pick-up") + } + if len(ghSrc.ExcludeComments) != 1 || ghSrc.ExcludeComments[0] != "/axon needs-input" { + t.Errorf("ExcludeComments = %v, want %v", ghSrc.ExcludeComments, []string{"/axon needs-input"}) + } +} diff --git a/install-crd.yaml b/install-crd.yaml index 9a64beff..1f164a2d 100644 --- a/install-crd.yaml +++ b/install-crd.yaml @@ -1084,6 +1084,16 @@ spec: githubIssues: description: GitHubIssues discovers issues from a GitHub repository. properties: + excludeComments: + description: |- + ExcludeComments enables comment-based exclusion. When set, issues that + have a comment matching any of these strings (e.g., "/axon needs-input") + are excluded unless a subsequent TriggerComment overrides it. Comments + are scanned in reverse chronological order — the most recent matching + command wins. + items: + type: string + type: array excludeLabels: description: ExcludeLabels filters out issues that have any of these labels (client-side). @@ -1112,6 +1122,16 @@ spec: - closed - all type: string + triggerComment: + description: |- + TriggerComment enables comment-based discovery. When set, only issues + that have a comment matching this string (e.g., "/axon pick-up") are + included. This is useful for repos where you lack label permissions. + If ExcludeComments is also set, TriggerComment doubles as a resume + command — the most recent match between TriggerComment and + ExcludeComments wins. Comments are scanned in reverse chronological + order. + type: string types: default: - issues diff --git a/internal/manifests/install-crd.yaml b/internal/manifests/install-crd.yaml index 9a64beff..1f164a2d 100644 --- a/internal/manifests/install-crd.yaml +++ b/internal/manifests/install-crd.yaml @@ -1084,6 +1084,16 @@ spec: githubIssues: description: GitHubIssues discovers issues from a GitHub repository. properties: + excludeComments: + description: |- + ExcludeComments enables comment-based exclusion. When set, issues that + have a comment matching any of these strings (e.g., "/axon needs-input") + are excluded unless a subsequent TriggerComment overrides it. Comments + are scanned in reverse chronological order — the most recent matching + command wins. + items: + type: string + type: array excludeLabels: description: ExcludeLabels filters out issues that have any of these labels (client-side). @@ -1112,6 +1122,16 @@ spec: - closed - all type: string + triggerComment: + description: |- + TriggerComment enables comment-based discovery. When set, only issues + that have a comment matching this string (e.g., "/axon pick-up") are + included. This is useful for repos where you lack label permissions. + If ExcludeComments is also set, TriggerComment doubles as a resume + command — the most recent match between TriggerComment and + ExcludeComments wins. Comments are scanned in reverse chronological + order. + type: string types: default: - issues diff --git a/internal/source/github.go b/internal/source/github.go index 39477715..3a9cd468 100644 --- a/internal/source/github.go +++ b/internal/source/github.go @@ -25,15 +25,17 @@ const ( // GitHubSource discovers issues from a GitHub repository. type GitHubSource struct { - Owner string - Repo string - Types []string - Labels []string - ExcludeLabels []string - State string - Token string - BaseURL string - Client *http.Client + Owner string + Repo string + Types []string + Labels []string + ExcludeLabels []string + State string + Token string + BaseURL string + Client *http.Client + TriggerComment string + ExcludeComments []string } type githubIssue struct { @@ -76,6 +78,8 @@ func (s *GitHubSource) Discover(ctx context.Context) ([]WorkItem, error) { issues = s.filterItems(issues) + needsCommentFilter := s.TriggerComment != "" || len(s.ExcludeComments) > 0 + var items []WorkItem for _, issue := range issues { var labels []string @@ -88,6 +92,10 @@ func (s *GitHubSource) Discover(ctx context.Context) ([]WorkItem, error) { return nil, fmt.Errorf("fetching comments for issue #%d: %w", issue.Number, err) } + if needsCommentFilter && !s.passesCommentFilter(comments) { + continue + } + kind := "Issue" if issue.PullRequest != nil { kind = "PR" @@ -108,6 +116,81 @@ func (s *GitHubSource) Discover(ctx context.Context) ([]WorkItem, error) { return items, nil } +// passesCommentFilter checks whether an issue's comments satisfy the +// comment-based trigger and exclude rules. Comments are expected in the +// concatenated format produced by fetchComments (separated by "\n---\n"). +// +// When both TriggerComment and ExcludeComments are set, the most recent +// matching command wins (scanned in reverse chronological order). +// TriggerComment doubles as a resume command — posting it after an +// ExcludeComment re-enables the issue. +func (s *GitHubSource) passesCommentFilter(comments string) bool { + // Split into individual comment bodies. + var parts []string + if comments != "" { + parts = strings.Split(comments, "\n---\n") + } + + // When only TriggerComment is set, require at least one matching comment. + if s.TriggerComment != "" && len(s.ExcludeComments) == 0 { + for _, p := range parts { + if containsCommand(p, s.TriggerComment) { + return true + } + } + return false + } + + // When only ExcludeComments is set, exclude if any comment matches. + if len(s.ExcludeComments) > 0 && s.TriggerComment == "" { + for i := len(parts) - 1; i >= 0; i-- { + if containsAnyCommand(parts[i], s.ExcludeComments) { + return false + } + } + return true + } + + // When both are set, scan in reverse; the most recent matching command wins. + // TriggerComment acts as both initial trigger and resume. + if s.TriggerComment != "" && len(s.ExcludeComments) > 0 { + for i := len(parts) - 1; i >= 0; i-- { + if containsAnyCommand(parts[i], s.ExcludeComments) { + return false + } + if containsCommand(parts[i], s.TriggerComment) { + return true + } + } + // Neither command found — trigger is required but absent. + return false + } + + return true +} + +// containsAnyCommand reports whether body contains any of the given commands. +func containsAnyCommand(body string, cmds []string) bool { + for _, cmd := range cmds { + if containsCommand(body, cmd) { + return true + } + } + return false +} + +// containsCommand reports whether body contains the given command string. +// The command must appear at the start of a line to avoid false matches +// inside prose. +func containsCommand(body, cmd string) bool { + for _, line := range strings.Split(body, "\n") { + if strings.TrimSpace(line) == cmd { + return true + } + } + return false +} + func (s *GitHubSource) resolvedTypes() map[string]struct{} { types := s.Types if len(types) == 0 { diff --git a/internal/source/github_test.go b/internal/source/github_test.go index 5af2d585..e6144401 100644 --- a/internal/source/github_test.go +++ b/internal/source/github_test.go @@ -516,3 +516,350 @@ func TestDiscoverTypesDefault(t *testing.T) { func containsParam(query, param string) bool { return strings.Contains(query, param) } + +func TestDiscoverTriggerComment(t *testing.T) { + issues := []githubIssue{ + {Number: 1, Title: "Triggered", Body: "Body 1", HTMLURL: "https://github.com/o/r/issues/1"}, + {Number: 2, Title: "Not triggered", Body: "Body 2", HTMLURL: "https://github.com/o/r/issues/2"}, + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/repos/owner/repo/issues": + json.NewEncoder(w).Encode(issues) + case r.URL.Path == "/repos/owner/repo/issues/1/comments": + json.NewEncoder(w).Encode([]githubComment{{Body: "/axon pick-up"}}) + case r.URL.Path == "/repos/owner/repo/issues/2/comments": + json.NewEncoder(w).Encode([]githubComment{{Body: "Just a regular comment"}}) + } + })) + defer server.Close() + + s := &GitHubSource{ + Owner: "owner", + Repo: "repo", + BaseURL: server.URL, + TriggerComment: "/axon pick-up", + } + + items, err := s.Discover(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(items) != 1 { + t.Fatalf("expected 1 item, got %d", len(items)) + } + if items[0].Number != 1 { + t.Errorf("expected issue #1, got #%d", items[0].Number) + } +} + +func TestDiscoverExcludeComment(t *testing.T) { + issues := []githubIssue{ + {Number: 1, Title: "Active", Body: "Body 1", HTMLURL: "https://github.com/o/r/issues/1"}, + {Number: 2, Title: "Needs input", Body: "Body 2", HTMLURL: "https://github.com/o/r/issues/2"}, + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/repos/owner/repo/issues": + json.NewEncoder(w).Encode(issues) + case r.URL.Path == "/repos/owner/repo/issues/1/comments": + json.NewEncoder(w).Encode([]githubComment{{Body: "Normal comment"}}) + case r.URL.Path == "/repos/owner/repo/issues/2/comments": + json.NewEncoder(w).Encode([]githubComment{{Body: "/axon needs-input"}}) + } + })) + defer server.Close() + + s := &GitHubSource{ + Owner: "owner", + Repo: "repo", + BaseURL: server.URL, + ExcludeComments: []string{"/axon needs-input"}, + } + + items, err := s.Discover(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(items) != 1 { + t.Fatalf("expected 1 item, got %d", len(items)) + } + if items[0].Number != 1 { + t.Errorf("expected issue #1, got #%d", items[0].Number) + } +} + +func TestDiscoverMultipleExcludeComments(t *testing.T) { + issues := []githubIssue{ + {Number: 1, Title: "Active", Body: "Body 1", HTMLURL: "https://github.com/o/r/issues/1"}, + {Number: 2, Title: "Needs input", Body: "Body 2", HTMLURL: "https://github.com/o/r/issues/2"}, + {Number: 3, Title: "Paused", Body: "Body 3", HTMLURL: "https://github.com/o/r/issues/3"}, + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/repos/owner/repo/issues": + json.NewEncoder(w).Encode(issues) + case r.URL.Path == "/repos/owner/repo/issues/1/comments": + json.NewEncoder(w).Encode([]githubComment{{Body: "Normal comment"}}) + case r.URL.Path == "/repos/owner/repo/issues/2/comments": + json.NewEncoder(w).Encode([]githubComment{{Body: "/axon needs-input"}}) + case r.URL.Path == "/repos/owner/repo/issues/3/comments": + json.NewEncoder(w).Encode([]githubComment{{Body: "/axon pause"}}) + } + })) + defer server.Close() + + s := &GitHubSource{ + Owner: "owner", + Repo: "repo", + BaseURL: server.URL, + ExcludeComments: []string{"/axon needs-input", "/axon pause"}, + } + + items, err := s.Discover(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(items) != 1 { + t.Fatalf("expected 1 item, got %d", len(items)) + } + if items[0].Number != 1 { + t.Errorf("expected issue #1, got #%d", items[0].Number) + } +} + +func TestDiscoverTriggerAsResume(t *testing.T) { + issues := []githubIssue{ + {Number: 1, Title: "Resumed", Body: "Body 1", HTMLURL: "https://github.com/o/r/issues/1"}, + {Number: 2, Title: "Still excluded", Body: "Body 2", HTMLURL: "https://github.com/o/r/issues/2"}, + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/repos/owner/repo/issues": + json.NewEncoder(w).Encode(issues) + case r.URL.Path == "/repos/owner/repo/issues/1/comments": + // Trigger, then exclude, then trigger again — trigger is most recent, so issue should be included + json.NewEncoder(w).Encode([]githubComment{ + {Body: "/axon pick-up"}, + {Body: "/axon needs-input"}, + {Body: "/axon pick-up"}, + }) + case r.URL.Path == "/repos/owner/repo/issues/2/comments": + // Trigger then exclude — exclude is most recent, so issue should be excluded + json.NewEncoder(w).Encode([]githubComment{ + {Body: "/axon pick-up"}, + {Body: "/axon needs-input"}, + }) + } + })) + defer server.Close() + + s := &GitHubSource{ + Owner: "owner", + Repo: "repo", + BaseURL: server.URL, + TriggerComment: "/axon pick-up", + ExcludeComments: []string{"/axon needs-input"}, + } + + items, err := s.Discover(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(items) != 1 { + t.Fatalf("expected 1 item, got %d", len(items)) + } + if items[0].Number != 1 { + t.Errorf("expected issue #1, got #%d", items[0].Number) + } +} + +func TestDiscoverTriggerAndExcludeComment(t *testing.T) { + issues := []githubIssue{ + {Number: 1, Title: "Triggered and active", Body: "Body 1", HTMLURL: "https://github.com/o/r/issues/1"}, + {Number: 2, Title: "Triggered but excluded", Body: "Body 2", HTMLURL: "https://github.com/o/r/issues/2"}, + {Number: 3, Title: "Not triggered", Body: "Body 3", HTMLURL: "https://github.com/o/r/issues/3"}, + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/repos/owner/repo/issues": + json.NewEncoder(w).Encode(issues) + case r.URL.Path == "/repos/owner/repo/issues/1/comments": + json.NewEncoder(w).Encode([]githubComment{{Body: "/axon pick-up"}}) + case r.URL.Path == "/repos/owner/repo/issues/2/comments": + json.NewEncoder(w).Encode([]githubComment{ + {Body: "/axon pick-up"}, + {Body: "/axon needs-input"}, + }) + case r.URL.Path == "/repos/owner/repo/issues/3/comments": + json.NewEncoder(w).Encode([]githubComment{{Body: "Just a comment"}}) + } + })) + defer server.Close() + + s := &GitHubSource{ + Owner: "owner", + Repo: "repo", + BaseURL: server.URL, + TriggerComment: "/axon pick-up", + ExcludeComments: []string{"/axon needs-input"}, + } + + items, err := s.Discover(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(items) != 1 { + t.Fatalf("expected 1 item, got %d", len(items)) + } + if items[0].Number != 1 { + t.Errorf("expected issue #1, got #%d", items[0].Number) + } +} + +func TestPassesCommentFilter(t *testing.T) { + tests := []struct { + name string + triggerComment string + excludeComments []string + comments string + want bool + }{ + { + name: "no filters configured", + comments: "some comment", + want: true, + }, + { + name: "trigger present", + triggerComment: "/axon pick-up", + comments: "/axon pick-up", + want: true, + }, + { + name: "trigger absent", + triggerComment: "/axon pick-up", + comments: "no trigger here", + want: false, + }, + { + name: "trigger empty comments", + triggerComment: "/axon pick-up", + comments: "", + want: false, + }, + { + name: "exclude present", + excludeComments: []string{"/axon needs-input"}, + comments: "/axon needs-input", + want: false, + }, + { + name: "exclude absent", + excludeComments: []string{"/axon needs-input"}, + comments: "normal comment", + want: true, + }, + { + name: "trigger as resume after exclude", + triggerComment: "/axon pick-up", + excludeComments: []string{"/axon needs-input"}, + comments: "/axon pick-up\n---\n/axon needs-input\n---\n/axon pick-up", + want: true, + }, + { + name: "exclude after trigger", + triggerComment: "/axon pick-up", + excludeComments: []string{"/axon needs-input"}, + comments: "/axon pick-up\n---\n/axon needs-input", + want: false, + }, + { + name: "both set but neither found", + triggerComment: "/axon pick-up", + excludeComments: []string{"/axon needs-input"}, + comments: "normal comment", + want: false, + }, + { + name: "command must be on its own line", + excludeComments: []string{"/axon needs-input"}, + comments: "please do /axon needs-input for me", + want: true, + }, + { + name: "multiple exclude comments", + excludeComments: []string{"/axon needs-input", "/axon pause"}, + comments: "/axon pause", + want: false, + }, + { + name: "multiple exclude comments none match", + excludeComments: []string{"/axon needs-input", "/axon pause"}, + comments: "normal comment", + want: true, + }, + { + name: "multiple exclude with trigger resume", + triggerComment: "/axon pick-up", + excludeComments: []string{"/axon needs-input", "/axon pause"}, + comments: "/axon pick-up\n---\n/axon pause\n---\n/axon pick-up", + want: true, + }, + { + name: "multiple exclude second matches most recent", + triggerComment: "/axon pick-up", + excludeComments: []string{"/axon needs-input", "/axon pause"}, + comments: "/axon pick-up\n---\n/axon pause", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &GitHubSource{ + TriggerComment: tt.triggerComment, + ExcludeComments: tt.excludeComments, + } + got := s.passesCommentFilter(tt.comments) + if got != tt.want { + t.Errorf("passesCommentFilter() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestContainsCommand(t *testing.T) { + tests := []struct { + name string + body string + cmd string + want bool + }{ + {"exact match", "/axon pick-up", "/axon pick-up", true}, + {"with whitespace", " /axon pick-up ", "/axon pick-up", true}, + {"multiline match", "some text\n/axon pick-up\nmore text", "/axon pick-up", true}, + {"no match", "some text without command", "/axon pick-up", false}, + {"partial match in word", "do /axon pick-up now", "/axon pick-up", false}, + {"empty body", "", "/axon pick-up", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := containsCommand(tt.body, tt.cmd) + if got != tt.want { + t.Errorf("containsCommand(%q, %q) = %v, want %v", tt.body, tt.cmd, got, tt.want) + } + }) + } +} diff --git a/test/integration/taskspawner_test.go b/test/integration/taskspawner_test.go index 4061cc5b..3b646c10 100644 --- a/test/integration/taskspawner_test.go +++ b/test/integration/taskspawner_test.go @@ -1484,4 +1484,224 @@ var _ = Describe("TaskSpawner Controller", func() { Expect(container.Args).NotTo(ContainElement("--github-owner=my-fork")) }) }) + + Context("When creating a TaskSpawner with comment-based workflow control", func() { + It("Should store triggerComment and excludeComments in spec and create a Deployment", func() { + By("Creating a namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-taskspawner-comments", + }, + } + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + + By("Creating a Workspace") + ws := &axonv1alpha1.Workspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-workspace-comments", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.WorkspaceSpec{ + Repo: "https://github.com/axon-core/axon.git", + Ref: "main", + }, + } + Expect(k8sClient.Create(ctx, ws)).Should(Succeed()) + + By("Creating a TaskSpawner with triggerComment and excludeComments") + ts := &axonv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-spawner-comments", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.TaskSpawnerSpec{ + When: axonv1alpha1.When{ + GitHubIssues: &axonv1alpha1.GitHubIssues{ + State: "open", + TriggerComment: "/axon pick-up", + ExcludeComments: []string{"/axon needs-input", "/axon pause"}, + }, + }, + TaskTemplate: axonv1alpha1.TaskTemplate{ + Type: "claude-code", + Credentials: axonv1alpha1.Credentials{ + Type: axonv1alpha1.CredentialTypeOAuth, + SecretRef: axonv1alpha1.SecretReference{ + Name: "claude-credentials", + }, + }, + WorkspaceRef: &axonv1alpha1.WorkspaceReference{ + Name: "test-workspace-comments", + }, + }, + PollInterval: "5m", + }, + } + Expect(k8sClient.Create(ctx, ts)).Should(Succeed()) + + By("Verifying the comment fields are stored in spec") + tsLookupKey := types.NamespacedName{Name: ts.Name, Namespace: ns.Name} + createdTS := &axonv1alpha1.TaskSpawner{} + Eventually(func() bool { + err := k8sClient.Get(ctx, tsLookupKey, createdTS) + return err == nil + }, timeout, interval).Should(BeTrue()) + Expect(createdTS.Spec.When.GitHubIssues.TriggerComment).To(Equal("/axon pick-up")) + Expect(createdTS.Spec.When.GitHubIssues.ExcludeComments).To(ConsistOf("/axon needs-input", "/axon pause")) + + By("Verifying a Deployment is created") + deployLookupKey := types.NamespacedName{Name: ts.Name, Namespace: ns.Name} + createdDeploy := &appsv1.Deployment{} + Eventually(func() bool { + err := k8sClient.Get(ctx, deployLookupKey, createdDeploy) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Verifying the Deployment has owner reference") + Expect(createdDeploy.OwnerReferences).To(HaveLen(1)) + Expect(createdDeploy.OwnerReferences[0].Name).To(Equal(ts.Name)) + }) + + It("Should store only triggerComment when excludeComments is not set", func() { + By("Creating a namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-taskspawner-trigger-only", + }, + } + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + + By("Creating a Workspace") + ws := &axonv1alpha1.Workspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-workspace-trigger-only", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.WorkspaceSpec{ + Repo: "https://github.com/axon-core/axon.git", + Ref: "main", + }, + } + Expect(k8sClient.Create(ctx, ws)).Should(Succeed()) + + By("Creating a TaskSpawner with only triggerComment") + ts := &axonv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-spawner-trigger-only", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.TaskSpawnerSpec{ + When: axonv1alpha1.When{ + GitHubIssues: &axonv1alpha1.GitHubIssues{ + State: "open", + TriggerComment: "/axon pick-up", + }, + }, + TaskTemplate: axonv1alpha1.TaskTemplate{ + Type: "claude-code", + Credentials: axonv1alpha1.Credentials{ + Type: axonv1alpha1.CredentialTypeOAuth, + SecretRef: axonv1alpha1.SecretReference{ + Name: "claude-credentials", + }, + }, + WorkspaceRef: &axonv1alpha1.WorkspaceReference{ + Name: "test-workspace-trigger-only", + }, + }, + PollInterval: "5m", + }, + } + Expect(k8sClient.Create(ctx, ts)).Should(Succeed()) + + By("Verifying triggerComment is stored and excludeComments is empty") + tsLookupKey := types.NamespacedName{Name: ts.Name, Namespace: ns.Name} + createdTS := &axonv1alpha1.TaskSpawner{} + Eventually(func() bool { + err := k8sClient.Get(ctx, tsLookupKey, createdTS) + return err == nil + }, timeout, interval).Should(BeTrue()) + Expect(createdTS.Spec.When.GitHubIssues.TriggerComment).To(Equal("/axon pick-up")) + Expect(createdTS.Spec.When.GitHubIssues.ExcludeComments).To(BeEmpty()) + + By("Verifying a Deployment is created") + deployLookupKey := types.NamespacedName{Name: ts.Name, Namespace: ns.Name} + createdDeploy := &appsv1.Deployment{} + Eventually(func() bool { + err := k8sClient.Get(ctx, deployLookupKey, createdDeploy) + return err == nil + }, timeout, interval).Should(BeTrue()) + }) + + It("Should store only excludeComments when triggerComment is not set", func() { + By("Creating a namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-taskspawner-exclude-only", + }, + } + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + + By("Creating a Workspace") + ws := &axonv1alpha1.Workspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-workspace-exclude-only", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.WorkspaceSpec{ + Repo: "https://github.com/axon-core/axon.git", + Ref: "main", + }, + } + Expect(k8sClient.Create(ctx, ws)).Should(Succeed()) + + By("Creating a TaskSpawner with only excludeComments") + ts := &axonv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-spawner-exclude-only", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.TaskSpawnerSpec{ + When: axonv1alpha1.When{ + GitHubIssues: &axonv1alpha1.GitHubIssues{ + State: "open", + ExcludeComments: []string{"/axon needs-input"}, + }, + }, + TaskTemplate: axonv1alpha1.TaskTemplate{ + Type: "claude-code", + Credentials: axonv1alpha1.Credentials{ + Type: axonv1alpha1.CredentialTypeOAuth, + SecretRef: axonv1alpha1.SecretReference{ + Name: "claude-credentials", + }, + }, + WorkspaceRef: &axonv1alpha1.WorkspaceReference{ + Name: "test-workspace-exclude-only", + }, + }, + PollInterval: "5m", + }, + } + Expect(k8sClient.Create(ctx, ts)).Should(Succeed()) + + By("Verifying excludeComments is stored and triggerComment is empty") + tsLookupKey := types.NamespacedName{Name: ts.Name, Namespace: ns.Name} + createdTS := &axonv1alpha1.TaskSpawner{} + Eventually(func() bool { + err := k8sClient.Get(ctx, tsLookupKey, createdTS) + return err == nil + }, timeout, interval).Should(BeTrue()) + Expect(createdTS.Spec.When.GitHubIssues.TriggerComment).To(BeEmpty()) + Expect(createdTS.Spec.When.GitHubIssues.ExcludeComments).To(ConsistOf("/axon needs-input")) + + By("Verifying a Deployment is created") + deployLookupKey := types.NamespacedName{Name: ts.Name, Namespace: ns.Name} + createdDeploy := &appsv1.Deployment{} + Eventually(func() bool { + err := k8sClient.Get(ctx, deployLookupKey, createdDeploy) + return err == nil + }, timeout, interval).Should(BeTrue()) + }) + }) }) From 33e2f6f26dc15046ad339c39776488709bd7ca1e Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Wed, 25 Feb 2026 20:55:53 +0000 Subject: [PATCH 4/4] Add integration tests for MCP server secret resolution Adds envtest-based integration tests covering headersFrom/envFrom secret resolution in the Task controller, as requested in review. Tests cover: - headersFrom: secret headers merged into AXON_MCP_SERVERS JSON - envFrom: secret env vars merged into AXON_MCP_SERVERS JSON - Missing secret causes Task to fail with MCPSecretFailed event - Secret values take precedence over inline values for overlapping keys Co-Authored-By: Claude Opus 4.6 --- test/integration/task_test.go | 415 ++++++++++++++++++++++++++++++++++ 1 file changed, 415 insertions(+) diff --git a/test/integration/task_test.go b/test/integration/task_test.go index 699ce932..167bef0e 100644 --- a/test/integration/task_test.go +++ b/test/integration/task_test.go @@ -3097,4 +3097,419 @@ var _ = Describe("Task Controller", func() { } }) }) + + Context("When creating a Task with AgentConfig MCP servers using headersFrom", func() { + It("Should resolve secret headers and include them in AXON_MCP_SERVERS", func() { + By("Creating a namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-mcp-headersfrom", + }, + } + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + + By("Creating a Secret with API key") + apiSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "anthropic-api-key", + Namespace: ns.Name, + }, + StringData: map[string]string{ + "ANTHROPIC_API_KEY": "test-api-key", + }, + } + Expect(k8sClient.Create(ctx, apiSecret)).Should(Succeed()) + + By("Creating a Secret with MCP headers") + mcpSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-github-headers", + Namespace: ns.Name, + }, + StringData: map[string]string{ + "Authorization": "Bearer secret-token", + "X-From-Secret": "secret-value", + }, + } + Expect(k8sClient.Create(ctx, mcpSecret)).Should(Succeed()) + + By("Creating an AgentConfig with MCP server using headersFrom") + agentConfig := &axonv1alpha1.AgentConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-headersfrom-config", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.AgentConfigSpec{ + MCPServers: []axonv1alpha1.MCPServerSpec{ + { + Name: "github", + Type: "http", + URL: "https://api.example.com/mcp/", + Headers: map[string]string{ + "X-Inline": "inline-value", + }, + HeadersFrom: &axonv1alpha1.SecretValuesSource{ + SecretRef: axonv1alpha1.SecretReference{Name: "mcp-github-headers"}, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, agentConfig)).Should(Succeed()) + + By("Creating a Task referencing the AgentConfig") + task := &axonv1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-mcp-headersfrom", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.TaskSpec{ + Type: "claude-code", + Prompt: "Test MCP headersFrom", + Credentials: axonv1alpha1.Credentials{ + Type: axonv1alpha1.CredentialTypeAPIKey, + SecretRef: axonv1alpha1.SecretReference{Name: "anthropic-api-key"}, + }, + AgentConfigRef: &axonv1alpha1.AgentConfigReference{Name: "mcp-headersfrom-config"}, + }, + } + Expect(k8sClient.Create(ctx, task)).Should(Succeed()) + + By("Verifying a Job is created") + createdJob := &batchv1.Job{} + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: task.Name, Namespace: ns.Name}, createdJob) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Logging the Job spec") + logJobSpec(createdJob) + + By("Verifying AXON_MCP_SERVERS contains resolved headers from secret") + container := createdJob.Spec.Template.Spec.Containers[0] + var mcpJSON string + for _, env := range container.Env { + if env.Name == "AXON_MCP_SERVERS" { + mcpJSON = env.Value + } + } + Expect(mcpJSON).NotTo(BeEmpty(), "Expected AXON_MCP_SERVERS env var") + + var parsed struct { + MCPServers map[string]struct { + Headers map[string]string `json:"headers"` + } `json:"mcpServers"` + } + Expect(json.Unmarshal([]byte(mcpJSON), &parsed)).Should(Succeed()) + + github, ok := parsed.MCPServers["github"] + Expect(ok).To(BeTrue(), "Expected 'github' MCP server entry") + Expect(github.Headers["Authorization"]).To(Equal("Bearer secret-token")) + Expect(github.Headers["X-From-Secret"]).To(Equal("secret-value")) + Expect(github.Headers["X-Inline"]).To(Equal("inline-value")) + }) + }) + + Context("When creating a Task with AgentConfig MCP servers using envFrom", func() { + It("Should resolve secret env vars and include them in AXON_MCP_SERVERS", func() { + By("Creating a namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-mcp-envfrom", + }, + } + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + + By("Creating a Secret with API key") + apiSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "anthropic-api-key", + Namespace: ns.Name, + }, + StringData: map[string]string{ + "ANTHROPIC_API_KEY": "test-api-key", + }, + } + Expect(k8sClient.Create(ctx, apiSecret)).Should(Succeed()) + + By("Creating a Secret with MCP env vars") + mcpSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-db-env", + Namespace: ns.Name, + }, + StringData: map[string]string{ + "DB_PASSWORD": "secret-pass", + "DB_HOST": "db.internal", + }, + } + Expect(k8sClient.Create(ctx, mcpSecret)).Should(Succeed()) + + By("Creating an AgentConfig with MCP server using envFrom") + agentConfig := &axonv1alpha1.AgentConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-envfrom-config", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.AgentConfigSpec{ + MCPServers: []axonv1alpha1.MCPServerSpec{ + { + Name: "local-db", + Type: "stdio", + Command: "npx", + Args: []string{"-y", "dbhub"}, + Env: map[string]string{ + "DSN": "postgres://localhost/db", + }, + EnvFrom: &axonv1alpha1.SecretValuesSource{ + SecretRef: axonv1alpha1.SecretReference{Name: "mcp-db-env"}, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, agentConfig)).Should(Succeed()) + + By("Creating a Task referencing the AgentConfig") + task := &axonv1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-mcp-envfrom", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.TaskSpec{ + Type: "claude-code", + Prompt: "Test MCP envFrom", + Credentials: axonv1alpha1.Credentials{ + Type: axonv1alpha1.CredentialTypeAPIKey, + SecretRef: axonv1alpha1.SecretReference{Name: "anthropic-api-key"}, + }, + AgentConfigRef: &axonv1alpha1.AgentConfigReference{Name: "mcp-envfrom-config"}, + }, + } + Expect(k8sClient.Create(ctx, task)).Should(Succeed()) + + By("Verifying a Job is created") + createdJob := &batchv1.Job{} + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: task.Name, Namespace: ns.Name}, createdJob) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Logging the Job spec") + logJobSpec(createdJob) + + By("Verifying AXON_MCP_SERVERS contains resolved env vars from secret") + container := createdJob.Spec.Template.Spec.Containers[0] + var mcpJSON string + for _, env := range container.Env { + if env.Name == "AXON_MCP_SERVERS" { + mcpJSON = env.Value + } + } + Expect(mcpJSON).NotTo(BeEmpty(), "Expected AXON_MCP_SERVERS env var") + + var parsed struct { + MCPServers map[string]struct { + Env map[string]string `json:"env"` + } `json:"mcpServers"` + } + Expect(json.Unmarshal([]byte(mcpJSON), &parsed)).Should(Succeed()) + + localDB, ok := parsed.MCPServers["local-db"] + Expect(ok).To(BeTrue(), "Expected 'local-db' MCP server entry") + Expect(localDB.Env["DB_PASSWORD"]).To(Equal("secret-pass")) + Expect(localDB.Env["DB_HOST"]).To(Equal("db.internal")) + Expect(localDB.Env["DSN"]).To(Equal("postgres://localhost/db")) + }) + }) + + Context("When creating a Task with MCP server referencing a missing secret", func() { + It("Should fail the Task with an appropriate error", func() { + By("Creating a namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-mcp-missing-secret", + }, + } + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + + By("Creating a Secret with API key") + apiSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "anthropic-api-key", + Namespace: ns.Name, + }, + StringData: map[string]string{ + "ANTHROPIC_API_KEY": "test-api-key", + }, + } + Expect(k8sClient.Create(ctx, apiSecret)).Should(Succeed()) + + By("Creating an AgentConfig referencing a nonexistent secret") + agentConfig := &axonv1alpha1.AgentConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-missing-secret-config", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.AgentConfigSpec{ + MCPServers: []axonv1alpha1.MCPServerSpec{ + { + Name: "api", + Type: "http", + URL: "https://api.example.com/mcp/", + HeadersFrom: &axonv1alpha1.SecretValuesSource{ + SecretRef: axonv1alpha1.SecretReference{Name: "nonexistent-secret"}, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, agentConfig)).Should(Succeed()) + + By("Creating a Task referencing the AgentConfig") + task := &axonv1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-mcp-missing-secret", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.TaskSpec{ + Type: "claude-code", + Prompt: "Test missing MCP secret", + Credentials: axonv1alpha1.Credentials{ + Type: axonv1alpha1.CredentialTypeAPIKey, + SecretRef: axonv1alpha1.SecretReference{Name: "anthropic-api-key"}, + }, + AgentConfigRef: &axonv1alpha1.AgentConfigReference{Name: "mcp-missing-secret-config"}, + }, + } + Expect(k8sClient.Create(ctx, task)).Should(Succeed()) + + By("Verifying Task status transitions to Failed") + taskLookupKey := types.NamespacedName{Name: task.Name, Namespace: ns.Name} + createdTask := &axonv1alpha1.Task{} + Eventually(func() axonv1alpha1.TaskPhase { + err := k8sClient.Get(ctx, taskLookupKey, createdTask) + if err != nil { + return "" + } + return createdTask.Status.Phase + }, timeout, interval).Should(Equal(axonv1alpha1.TaskPhaseFailed)) + + By("Verifying error message mentions the missing secret") + Expect(createdTask.Status.Message).To(ContainSubstring("nonexistent-secret")) + + By("Verifying MCPSecretFailed event is emitted") + Eventually(func() *corev1.Event { + return findEvent(ns.Name, task.Name, "MCPSecretFailed") + }, timeout, interval).ShouldNot(BeNil()) + }) + }) + + Context("When creating a Task with MCP servers using headersFrom precedence over inline", func() { + It("Should give secret values precedence over inline values for overlapping keys", func() { + By("Creating a namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-mcp-precedence", + }, + } + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + + By("Creating a Secret with API key") + apiSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "anthropic-api-key", + Namespace: ns.Name, + }, + StringData: map[string]string{ + "ANTHROPIC_API_KEY": "test-api-key", + }, + } + Expect(k8sClient.Create(ctx, apiSecret)).Should(Succeed()) + + By("Creating a Secret with an overlapping header key") + mcpSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-override-headers", + Namespace: ns.Name, + }, + StringData: map[string]string{ + "Authorization": "Bearer from-secret", + }, + } + Expect(k8sClient.Create(ctx, mcpSecret)).Should(Succeed()) + + By("Creating an AgentConfig with both inline and secretRef for the same header") + agentConfig := &axonv1alpha1.AgentConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-precedence-config", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.AgentConfigSpec{ + MCPServers: []axonv1alpha1.MCPServerSpec{ + { + Name: "api", + Type: "http", + URL: "https://api.example.com/mcp/", + Headers: map[string]string{ + "Authorization": "Bearer inline-token", + "X-Only-Inline": "preserved", + }, + HeadersFrom: &axonv1alpha1.SecretValuesSource{ + SecretRef: axonv1alpha1.SecretReference{Name: "mcp-override-headers"}, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, agentConfig)).Should(Succeed()) + + By("Creating a Task referencing the AgentConfig") + task := &axonv1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-mcp-precedence", + Namespace: ns.Name, + }, + Spec: axonv1alpha1.TaskSpec{ + Type: "claude-code", + Prompt: "Test MCP precedence", + Credentials: axonv1alpha1.Credentials{ + Type: axonv1alpha1.CredentialTypeAPIKey, + SecretRef: axonv1alpha1.SecretReference{Name: "anthropic-api-key"}, + }, + AgentConfigRef: &axonv1alpha1.AgentConfigReference{Name: "mcp-precedence-config"}, + }, + } + Expect(k8sClient.Create(ctx, task)).Should(Succeed()) + + By("Verifying a Job is created") + createdJob := &batchv1.Job{} + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: task.Name, Namespace: ns.Name}, createdJob) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Verifying secret value takes precedence over inline for overlapping key") + container := createdJob.Spec.Template.Spec.Containers[0] + var mcpJSON string + for _, env := range container.Env { + if env.Name == "AXON_MCP_SERVERS" { + mcpJSON = env.Value + } + } + Expect(mcpJSON).NotTo(BeEmpty(), "Expected AXON_MCP_SERVERS env var") + + var parsed struct { + MCPServers map[string]struct { + Headers map[string]string `json:"headers"` + } `json:"mcpServers"` + } + Expect(json.Unmarshal([]byte(mcpJSON), &parsed)).Should(Succeed()) + + api, ok := parsed.MCPServers["api"] + Expect(ok).To(BeTrue(), "Expected 'api' MCP server entry") + Expect(api.Headers["Authorization"]).To(Equal("Bearer from-secret"), "Secret value should take precedence") + Expect(api.Headers["X-Only-Inline"]).To(Equal("preserved"), "Non-overlapping inline header should be preserved") + }) + }) })