diff --git a/go/core/internal/controller/agentobject_helpers.go b/go/core/internal/controller/agentobject_helpers.go index 8b1a25c07..c51992810 100644 --- a/go/core/internal/controller/agentobject_helpers.go +++ b/go/core/internal/controller/agentobject_helpers.go @@ -78,6 +78,14 @@ func usesModelConfig(agent *v1alpha3.SandboxAgent, obj types.NamespacedName) boo spec.Declarative.ModelConfig == obj.Name } +func valueRefsReferenceSecret(refs []v1alpha3.ValueRef, name string) bool { + return slices.ContainsFunc(refs, func(ref v1alpha3.ValueRef) bool { + return ref.ValueFrom != nil && + ref.ValueFrom.Type == v1alpha3.SecretValueSource && + ref.ValueFrom.Name == name + }) +} + func referencesConfigMap(agent *v1alpha3.SandboxAgent, obj types.NamespacedName) bool { spec := agent.GetAgentSpec() if agent.GetNamespace() != obj.Namespace || spec.Type != v1alpha3.AgentType_Declarative || spec.Declarative == nil { diff --git a/go/core/internal/controller/remote_mcp_server_controller.go b/go/core/internal/controller/remote_mcp_server_controller.go index 73987a38d..cf5cc0f76 100644 --- a/go/core/internal/controller/remote_mcp_server_controller.go +++ b/go/core/internal/controller/remote_mcp_server_controller.go @@ -131,5 +131,10 @@ func remoteMCPServerReferencesSecret(server *v1alpha3.RemoteMCPServer, secretObj return true } + // check if secret is referenced by an upstream request header + if valueRefsReferenceSecret(server.Spec.HeadersFrom, secretObj.Name) { + return true + } + return false } diff --git a/go/core/internal/controller/sandboxagent_controller.go b/go/core/internal/controller/sandboxagent_controller.go index 9932b82af..2b82ee374 100644 --- a/go/core/internal/controller/sandboxagent_controller.go +++ b/go/core/internal/controller/sandboxagent_controller.go @@ -119,6 +119,7 @@ func (r *SandboxAgentController) SetupWithManager(mgr ctrl.Manager) error { mcpService: r.sandboxAgentDependencyFinder("failed to list sandboxagents for Service watch", usesMCPService), configMap: r.sandboxAgentDependencyFinder("failed to list sandboxagents for ConfigMap watch", referencesConfigMap), mcpServer: r.sandboxAgentDependencyFinder("failed to list sandboxagents for MCPServer watch", usesMCPServer), + secret: r.sandboxAgentSecretFinder, }) if err != nil { return err @@ -159,3 +160,77 @@ func (r *SandboxAgentController) sandboxAgentDependencyFinder(errMsg string, pre }) } } + +func (r *SandboxAgentController) sandboxAgentSecretFinder( + ctx context.Context, + cl client.Client, + secret types.NamespacedName, +) []types.NamespacedName { + var agents v1alpha3.SandboxAgentList + if err := cl.List(ctx, &agents); err != nil { + sandboxAgentControllerLog.Error(err, "failed to list SandboxAgents for Secret watch") + return nil + } + + matchedServers := make(map[types.NamespacedName]*v1alpha3.RemoteMCPServer) + var remoteMCPServers v1alpha3.RemoteMCPServerList + // Unlike sibling finders, don't fail closed here: a RemoteMCPServer list + // failure must not drop the direct tool.HeadersFrom matches below, which + // never depended on that list. Degrade to indirect-incomplete instead. + if err := cl.List(ctx, &remoteMCPServers); err != nil { + sandboxAgentControllerLog.Error(err, "failed to list RemoteMCPServers for Secret watch; indirect matches may be incomplete") + } else { + for i := range remoteMCPServers.Items { + server := &remoteMCPServers.Items[i] + if server.Namespace == secret.Namespace && + valueRefsReferenceSecret(server.Spec.HeadersFrom, secret.Name) { + matchedServers[types.NamespacedName{ + Name: server.Name, + Namespace: server.Namespace, + }] = server + } + } + } + + return collectSandboxAgentRefs(agents.Items, func(agent *v1alpha3.SandboxAgent) bool { + spec := agent.GetAgentSpec() + if spec.Type != v1alpha3.AgentType_Declarative || spec.Declarative == nil { + return false + } + + for _, tool := range spec.Declarative.Tools { + if tool == nil { + continue + } + if agent.Namespace == secret.Namespace && + valueRefsReferenceSecret(tool.HeadersFrom, secret.Name) { + return true + } + if tool.McpServer == nil || + (tool.McpServer.ApiGroup != "" && tool.McpServer.ApiGroup != "kagent.dev") || + tool.McpServer.Kind != "RemoteMCPServer" { + continue + } + + server, matched := matchedServers[tool.McpServer.NamespacedName(agent.Namespace)] + if !matched { + continue + } + allowed, err := server.Spec.AllowedNamespaces.AllowsNamespace( + ctx, + cl, + agent.Namespace, + server.Namespace, + ) + if err != nil { + sandboxAgentControllerLog.Error(err, "failed to check RemoteMCPServer namespace access") + continue + } + if allowed { + return true + } + } + + return false + }) +} diff --git a/go/core/internal/controller/sandboxagent_controller_test.go b/go/core/internal/controller/sandboxagent_controller_test.go new file mode 100644 index 000000000..843bb6b6a --- /dev/null +++ b/go/core/internal/controller/sandboxagent_controller_test.go @@ -0,0 +1,332 @@ +package controller + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/event" + + "github.com/kagent-dev/kagent/go/api/v1alpha3" +) + +func secretHeaderRef(name string) v1alpha3.ValueRef { + return v1alpha3.ValueRef{ + Name: "Authorization", + ValueFrom: &v1alpha3.ValueSource{ + Type: v1alpha3.SecretValueSource, + Name: name, + Key: "token", + }, + } +} + +func remoteMCPServerTool(name, namespace string) *v1alpha3.Tool { + return &v1alpha3.Tool{ + McpServer: &v1alpha3.McpServerTool{ + TypedReference: v1alpha3.TypedReference{ + ApiGroup: "kagent.dev", + Kind: "RemoteMCPServer", + Name: name, + Namespace: namespace, + }, + }, + } +} + +func remoteMCPServerToolEmptyGroup(name, namespace string) *v1alpha3.Tool { + return &v1alpha3.Tool{ + McpServer: &v1alpha3.McpServerTool{ + TypedReference: v1alpha3.TypedReference{ + Kind: "RemoteMCPServer", + Name: name, + Namespace: namespace, + }, + }, + } +} + +func declarativeSandboxAgent(name, namespace string, tools ...*v1alpha3.Tool) *v1alpha3.SandboxAgent { + return &v1alpha3.SandboxAgent{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + Spec: v1alpha3.AgentSpec{ + Type: v1alpha3.AgentType_Declarative, + Declarative: &v1alpha3.DeclarativeAgentSpec{ + Tools: tools, + }, + }, + } +} + +func TestSandboxAgentSecretFinder(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, v1alpha3.AddToScheme(scheme)) + + tests := []struct { + name string + secret types.NamespacedName + objects []client.Object + wantAgents []types.NamespacedName + }{ + { + name: "indirect RemoteMCPServer reference", + secret: types.NamespacedName{Name: "credentials", Namespace: "tools"}, + objects: []client.Object{ + &v1alpha3.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "remote", Namespace: "tools"}, + Spec: v1alpha3.RemoteMCPServerSpec{ + HeadersFrom: []v1alpha3.ValueRef{secretHeaderRef("credentials")}, + }, + }, + declarativeSandboxAgent( + "agent", + "tools", + remoteMCPServerTool("remote", ""), + ), + }, + wantAgents: []types.NamespacedName{{Name: "agent", Namespace: "tools"}}, + }, + { + name: "direct tool header reference", + secret: types.NamespacedName{Name: "credentials", Namespace: "agents"}, + objects: []client.Object{ + declarativeSandboxAgent( + "agent", + "agents", + &v1alpha3.Tool{ + HeadersFrom: []v1alpha3.ValueRef{secretHeaderRef("credentials")}, + }, + ), + }, + wantAgents: []types.NamespacedName{{Name: "agent", Namespace: "agents"}}, + }, + { + name: "cross namespace RemoteMCPServer allowed from all", + secret: types.NamespacedName{Name: "credentials", Namespace: "tools"}, + objects: []client.Object{ + &v1alpha3.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "remote", Namespace: "tools"}, + Spec: v1alpha3.RemoteMCPServerSpec{ + HeadersFrom: []v1alpha3.ValueRef{secretHeaderRef("credentials")}, + AllowedNamespaces: &v1alpha3.AllowedNamespaces{ + From: v1alpha3.NamespacesFromAll, + }, + }, + }, + declarativeSandboxAgent( + "agent", + "agents", + remoteMCPServerTool("remote", "tools"), + ), + }, + wantAgents: []types.NamespacedName{{Name: "agent", Namespace: "agents"}}, + }, + { + name: "Secret and RemoteMCPServer namespaces differ", + secret: types.NamespacedName{Name: "credentials", Namespace: "other"}, + objects: []client.Object{ + &v1alpha3.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "remote", Namespace: "tools"}, + Spec: v1alpha3.RemoteMCPServerSpec{ + HeadersFrom: []v1alpha3.ValueRef{secretHeaderRef("credentials")}, + AllowedNamespaces: &v1alpha3.AllowedNamespaces{ + From: v1alpha3.NamespacesFromAll, + }, + }, + }, + declarativeSandboxAgent( + "agent", + "agents", + remoteMCPServerTool("remote", "tools"), + ), + }, + }, + { + name: "cross namespace RemoteMCPServer denied by default", + secret: types.NamespacedName{Name: "credentials", Namespace: "tools"}, + objects: []client.Object{ + &v1alpha3.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "remote", Namespace: "tools"}, + Spec: v1alpha3.RemoteMCPServerSpec{ + HeadersFrom: []v1alpha3.ValueRef{secretHeaderRef("credentials")}, + }, + }, + declarativeSandboxAgent( + "agent", + "agents", + remoteMCPServerTool("remote", "tools"), + ), + }, + }, + { + name: "empty apiGroup same namespace RemoteMCPServer reference", + secret: types.NamespacedName{Name: "credentials", Namespace: "tools"}, + objects: []client.Object{ + &v1alpha3.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "remote", Namespace: "tools"}, + Spec: v1alpha3.RemoteMCPServerSpec{ + HeadersFrom: []v1alpha3.ValueRef{secretHeaderRef("credentials")}, + }, + }, + declarativeSandboxAgent( + "agent", + "tools", + remoteMCPServerToolEmptyGroup("remote", ""), + ), + }, + wantAgents: []types.NamespacedName{{Name: "agent", Namespace: "tools"}}, + }, + { + name: "empty apiGroup cross namespace RemoteMCPServer allowed from all", + secret: types.NamespacedName{Name: "credentials", Namespace: "tools"}, + objects: []client.Object{ + &v1alpha3.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "remote", Namespace: "tools"}, + Spec: v1alpha3.RemoteMCPServerSpec{ + HeadersFrom: []v1alpha3.ValueRef{secretHeaderRef("credentials")}, + AllowedNamespaces: &v1alpha3.AllowedNamespaces{ + From: v1alpha3.NamespacesFromAll, + }, + }, + }, + declarativeSandboxAgent( + "agent", + "agents", + remoteMCPServerToolEmptyGroup("remote", "tools"), + ), + }, + wantAgents: []types.NamespacedName{{Name: "agent", Namespace: "agents"}}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(tt.objects...). + Build() + + got := (&SandboxAgentController{}).sandboxAgentSecretFinder( + context.Background(), + cl, + tt.secret, + ) + + assert.ElementsMatch(t, tt.wantAgents, got) + }) + } +} + +func TestSandboxAgentSecretFinderReturnsDirectMatchesWhenRemoteMCPServerListFails(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, v1alpha3.AddToScheme(scheme)) + + agent := declarativeSandboxAgent( + "agent", + "agents", + &v1alpha3.Tool{ + HeadersFrom: []v1alpha3.ValueRef{secretHeaderRef("credentials")}, + }, + ) + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(agent). + WithInterceptorFuncs(interceptor.Funcs{ + List: func( + ctx context.Context, + c client.WithWatch, + list client.ObjectList, + opts ...client.ListOption, + ) error { + if _, ok := list.(*v1alpha3.RemoteMCPServerList); ok { + return fmt.Errorf("simulated RemoteMCPServer list failure") + } + return c.List(ctx, list, opts...) + }, + }). + Build() + + got := (&SandboxAgentController{}).sandboxAgentSecretFinder( + context.Background(), + cl, + types.NamespacedName{Name: "credentials", Namespace: "agents"}, + ) + + assert.Equal(t, []types.NamespacedName{{Name: "agent", Namespace: "agents"}}, got) +} + +func TestSecretDataChangeSelectsReferencingAgent(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, v1alpha3.AddToScheme(scheme)) + + oldSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "credentials", Namespace: "agents"}, + Data: map[string][]byte{"token": []byte("old")}, + } + newSecret := oldSecret.DeepCopy() + newSecret.Data["token"] = []byte("new") + + assert.True(t, secretDataChangedPredicate().Update(event.UpdateEvent{ + ObjectOld: oldSecret, + ObjectNew: newSecret, + })) + + agent := declarativeSandboxAgent( + "agent", + "agents", + &v1alpha3.Tool{ + HeadersFrom: []v1alpha3.ValueRef{secretHeaderRef("credentials")}, + }, + ) + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(agent). + Build() + + assert.Equal( + t, + []types.NamespacedName{{Name: "agent", Namespace: "agents"}}, + (&SandboxAgentController{}).sandboxAgentSecretFinder( + context.Background(), + cl, + types.NamespacedName{Name: "credentials", Namespace: "agents"}, + ), + ) + + unchanged := newSecret.DeepCopy() + assert.False(t, secretDataChangedPredicate().Update(event.UpdateEvent{ + ObjectOld: newSecret, + ObjectNew: unchanged, + })) +} + +func TestRemoteMCPServerReferencesHeaderSecret(t *testing.T) { + server := &v1alpha3.RemoteMCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "remote", Namespace: "tools"}, + Spec: v1alpha3.RemoteMCPServerSpec{ + HeadersFrom: []v1alpha3.ValueRef{secretHeaderRef("credentials")}, + }, + } + + assert.True(t, remoteMCPServerReferencesSecret( + server, + types.NamespacedName{Name: "credentials", Namespace: "tools"}, + )) + assert.False(t, remoteMCPServerReferencesSecret( + server, + types.NamespacedName{Name: "credentials", Namespace: "other"}, + )) +} diff --git a/go/core/internal/controller/watch_helpers.go b/go/core/internal/controller/watch_helpers.go index 124d8e631..16cbd6522 100644 --- a/go/core/internal/controller/watch_helpers.go +++ b/go/core/internal/controller/watch_helpers.go @@ -3,6 +3,7 @@ package controller import ( "context" "fmt" + "reflect" "github.com/kagent-dev/kagent/go/api/v1alpha3" "github.com/kagent-dev/kmcp/api/v1alpha1" @@ -38,6 +39,7 @@ type agentWatchFinders struct { mcpService dependentRefFinder configMap dependentRefFinder mcpServer dependentRefFinder + secret dependentRefFinder } func addOwnedResourceWatches(build *builder.Builder, mgr ctrl.Manager, owned []client.Object) (*builder.Builder, error) { @@ -93,6 +95,15 @@ func addCommonAgentWatches(build *builder.Builder, mgr ctrl.Manager, finders age })) }), builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), + ).Watches( + &corev1.Secret{}, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []reconcile.Request { + return reconcileRequestsForRefs(finders.secret(ctx, mgr.GetClient(), types.NamespacedName{ + Name: obj.GetName(), + Namespace: obj.GetNamespace(), + })) + }), + builder.WithPredicates(secretDataChangedPredicate()), ) if _, err := mgr.GetRESTMapper().RESTMapping(mcpServerGK); err == nil { @@ -112,3 +123,20 @@ func addCommonAgentWatches(build *builder.Builder, mgr ctrl.Manager, finders age return build, nil } + +func secretDataChangedPredicate() predicate.Predicate { + return predicate.Funcs{ + CreateFunc: func(event.CreateEvent) bool { return true }, + DeleteFunc: func(event.DeleteEvent) bool { return true }, + UpdateFunc: func(e event.UpdateEvent) bool { + oldSecret, oldOK := e.ObjectOld.(*corev1.Secret) + newSecret, newOK := e.ObjectNew.(*corev1.Secret) + if !oldOK || !newOK { + return true + } + + return !reflect.DeepEqual(oldSecret.Data, newSecret.Data) || + !reflect.DeepEqual(oldSecret.StringData, newSecret.StringData) + }, + } +}