diff --git a/acp/internal/server/server.go b/acp/internal/server/server.go index c705869..833317e 100644 --- a/acp/internal/server/server.go +++ b/acp/internal/server/server.go @@ -40,6 +40,13 @@ type CreateAgentRequest struct { MCPServers map[string]MCPServerConfig `json:"mcpServers,omitempty"` // Optional } +// UpdateAgentRequest defines the structure of the request body for updating an agent +type UpdateAgentRequest struct { + LLM string `json:"llm"` // Required + SystemPrompt string `json:"systemPrompt"` // Required + MCPServers map[string]MCPServerConfig `json:"mcpServers,omitempty"` // Optional +} + // MCPServerConfig defines the configuration for an MCP server type MCPServerConfig struct { Transport string `json:"transport"` // Required: "stdio" or "http" @@ -103,6 +110,7 @@ func (s *APIServer) registerRoutes() { agents.GET("", s.listAgents) agents.GET("/:name", s.getAgent) agents.POST("", s.createAgent) + agents.PUT("/:name", s.updateAgent) } // processMCPServers creates MCP servers and their secrets based on the given configuration @@ -618,6 +626,218 @@ func defaultIfEmpty(val, defaultVal string) string { return val } +// updateAgent handles updating an existing agent and its associated MCP servers +func (s *APIServer) updateAgent(c *gin.Context) { + ctx := c.Request.Context() + logger := log.FromContext(ctx) + + // Get namespace and name + namespace := c.Query("namespace") + if namespace == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "namespace query parameter is required"}) + return + } + name := c.Param("name") + if name == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "agent name is required"}) + return + } + + // Read the raw data for validation + var rawData []byte + if data, err := c.GetRawData(); err == nil { + rawData = data + } else { + c.JSON(http.StatusBadRequest, gin.H{"error": "Failed to read request body: " + err.Error()}) + return + } + + // Parse request + var req UpdateAgentRequest + if err := json.Unmarshal(rawData, &req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid request body: " + err.Error()}) + return + } + + // Validate for unknown fields + decoder := json.NewDecoder(bytes.NewReader(rawData)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&req); err != nil { + if strings.Contains(err.Error(), "unknown field") { + c.JSON(http.StatusBadRequest, gin.H{"error": "Unknown field in request: " + err.Error()}) + return + } + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid JSON format: " + err.Error()}) + return + } + + // Validate required fields + if req.LLM == "" || req.SystemPrompt == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "llm and systemPrompt are required"}) + return + } + + // Fetch current agent + var currentAgent acp.Agent + if err := s.client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, ¤tAgent); err != nil { + if apierrors.IsNotFound(err) { + c.JSON(http.StatusNotFound, gin.H{"error": "Agent not found"}) + return + } + logger.Error(err, "Failed to get agent", "name", name) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get agent: " + err.Error()}) + return + } + + // Verify LLM exists + if err := s.client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: req.LLM}, &acp.LLM{}); err != nil { + if apierrors.IsNotFound(err) { + c.JSON(http.StatusNotFound, gin.H{"error": "LLM not found"}) + return + } + logger.Error(err, "Failed to check LLM") + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to check LLM: " + err.Error()}) + return + } + + // Track current MCP servers for this agent + currentMCPServers := make(map[string]struct{}) + for _, ref := range currentAgent.Spec.MCPServers { + currentMCPServers[ref.Name] = struct{}{} + } + + // Process new/updated MCP servers + desiredMCPServers := make(map[string]MCPServerConfig) + for key, config := range req.MCPServers { + mcpName := fmt.Sprintf("%s-%s", name, key) + if err := validateMCPConfig(config); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("Invalid MCP server config for '%s': %s", key, err.Error())}) + return + } + desiredMCPServers[mcpName] = config + } + + // Create or update MCP servers + for mcpName, config := range desiredMCPServers { + secretName := fmt.Sprintf("%s-secrets", mcpName) + var mcpServer acp.MCPServer + err := s.client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: mcpName}, &mcpServer) + if apierrors.IsNotFound(err) { + // Create new MCP server and secret + if len(config.Secrets) > 0 { + secret := createSecret(secretName, namespace, config.Secrets) + if err := s.client.Create(ctx, secret); err != nil { + logger.Error(err, "Failed to create secret", "name", secretName) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create secret: " + err.Error()}) + return + } + } + mcpServer := createMCPServer(mcpName, namespace, config, secretName) + if err := s.client.Create(ctx, mcpServer); err != nil { + logger.Error(err, "Failed to create MCP server", "name", mcpName) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create MCP server: " + err.Error()}) + return + } + } else if err == nil { + // Update existing MCP server + updatedMCP := createMCPServer(mcpName, namespace, config, secretName) + updatedMCP.ObjectMeta = mcpServer.ObjectMeta + if err := s.client.Update(ctx, updatedMCP); err != nil { + logger.Error(err, "Failed to update MCP server", "name", mcpName) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update MCP server: " + err.Error()}) + return + } + // Handle secret + if len(config.Secrets) > 0 { + var secret corev1.Secret + err := s.client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: secretName}, &secret) + if apierrors.IsNotFound(err) { + secret := createSecret(secretName, namespace, config.Secrets) + if err := s.client.Create(ctx, secret); err != nil { + logger.Error(err, "Failed to create secret", "name", secretName) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create secret: " + err.Error()}) + return + } + } else if err == nil { + for k, v := range config.Secrets { + if secret.Data == nil { + secret.Data = make(map[string][]byte) + } + secret.Data[k] = []byte(v) + } + if err := s.client.Update(ctx, &secret); err != nil { + logger.Error(err, "Failed to update secret", "name", secretName) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update secret: " + err.Error()}) + return + } + } else { + logger.Error(err, "Failed to get secret", "name", secretName) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get secret: " + err.Error()}) + return + } + } else { + // Delete secret if it exists and no secrets are specified + var secret corev1.Secret + if err := s.client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: secretName}, &secret); err == nil { + if err := s.client.Delete(ctx, &secret); err != nil { + logger.Error(err, "Failed to delete secret", "name", secretName) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to delete secret: " + err.Error()}) + return + } + } + } + } else { + logger.Error(err, "Failed to get MCP server", "name", mcpName) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get MCP server: " + err.Error()}) + return + } + delete(currentMCPServers, mcpName) + } + + // Delete removed MCP servers + for mcpName := range currentMCPServers { + var mcpServer acp.MCPServer + if err := s.client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: mcpName}, &mcpServer); err == nil { + if err := s.client.Delete(ctx, &mcpServer); err != nil { + logger.Error(err, "Failed to delete MCP server", "name", mcpName) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to delete MCP server: " + err.Error()}) + return + } + } + secretName := fmt.Sprintf("%s-secrets", mcpName) + var secret corev1.Secret + if err := s.client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: secretName}, &secret); err == nil { + if err := s.client.Delete(ctx, &secret); err != nil { + logger.Error(err, "Failed to delete secret", "name", secretName) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to delete secret: " + err.Error()}) + return + } + } + } + + // Update agent spec + currentAgent.Spec.LLMRef = acp.LocalObjectReference{Name: req.LLM} + currentAgent.Spec.System = req.SystemPrompt + currentAgent.Spec.MCPServers = []acp.LocalObjectReference{} + for mcpName := range desiredMCPServers { + currentAgent.Spec.MCPServers = append(currentAgent.Spec.MCPServers, acp.LocalObjectReference{Name: mcpName}) + } + + if err := s.client.Update(ctx, ¤tAgent); err != nil { + logger.Error(err, "Failed to update agent", "name", name) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update agent: " + err.Error()}) + return + } + + c.JSON(http.StatusOK, AgentResponse{ + Namespace: namespace, + Name: name, + LLM: req.LLM, + SystemPrompt: req.SystemPrompt, + MCPServers: req.MCPServers, + }) +} + // createTask handles the creation of a new task func (s *APIServer) createTask(c *gin.Context) { ctx := c.Request.Context() diff --git a/acp/internal/server/server_test.go b/acp/internal/server/server_test.go index c34fefc..d4c7c20 100644 --- a/acp/internal/server/server_test.go +++ b/acp/internal/server/server_test.go @@ -13,6 +13,7 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -848,5 +849,333 @@ var _ = Describe("Agent API", func() { Expect(err).NotTo(HaveOccurred()) Expect(errorResponse["error"]).To(Equal("namespace query parameter is required")) }) + + Describe("PUT /v1/agents/:name", func() { + It("should update an existing agent", func() { + // Create an LLM + createTestLLM("test-llm-update", "default") + + // Create an agent first + agent := &acp.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "update-agent", + Namespace: "default", + }, + Spec: acp.AgentSpec{ + LLMRef: acp.LocalObjectReference{Name: "test-llm-update"}, + System: "Old prompt", + }, + } + Expect(k8sClient.Create(ctx, agent)).To(Succeed()) + + // Prepare the update request + reqBody := UpdateAgentRequest{ + LLM: "test-llm-update", + SystemPrompt: "New prompt", + } + jsonBody, err := json.Marshal(reqBody) + Expect(err).NotTo(HaveOccurred()) + + // Create a test request + req := httptest.NewRequest(http.MethodPut, "/v1/agents/update-agent?namespace=default", bytes.NewBuffer(jsonBody)) + req.Header.Set("Content-Type", "application/json") + recorder = httptest.NewRecorder() + + // Serve the request + router.ServeHTTP(recorder, req) + + // Verify the response + Expect(recorder.Code).To(Equal(http.StatusOK)) + + // Parse the response + var response AgentResponse + err = json.Unmarshal(recorder.Body.Bytes(), &response) + Expect(err).NotTo(HaveOccurred()) + + // Verify response has updated values + Expect(response.Name).To(Equal("update-agent")) + Expect(response.SystemPrompt).To(Equal("New prompt")) + + // Verify agent was updated in Kubernetes + var updatedAgent acp.Agent + Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: "update-agent", + Namespace: "default", + }, &updatedAgent)).To(Succeed()) + + Expect(updatedAgent.Spec.System).To(Equal("New prompt")) + }) + + It("should update MCP servers (add, update, remove)", func() { + // Create an LLM + createTestLLM("test-llm-mcp-update", "default") + + // Create an MCP server that will be removed during update + oldMCP := &acp.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-update-agent-old", + Namespace: "default", + }, + Spec: acp.MCPServerSpec{ + Transport: "stdio", + Command: "oldcmd", + Args: []string{"oldarg"}, + }, + } + Expect(k8sClient.Create(ctx, oldMCP)).To(Succeed()) + + // Create an agent with an initial MCP server + agent := &acp.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-update-agent", + Namespace: "default", + }, + Spec: acp.AgentSpec{ + LLMRef: acp.LocalObjectReference{Name: "test-llm-mcp-update"}, + System: "Original prompt", + MCPServers: []acp.LocalObjectReference{ + {Name: "mcp-update-agent-old"}, + }, + }, + } + Expect(k8sClient.Create(ctx, agent)).To(Succeed()) + + // Prepare update request with new MCP server configuration + reqBody := UpdateAgentRequest{ + LLM: "test-llm-mcp-update", + SystemPrompt: "Updated prompt", + MCPServers: map[string]MCPServerConfig{ + "new": { + Transport: "http", + URL: "http://newserver.com", + }, + "existing": { + Transport: "stdio", + Command: "updated-cmd", + Args: []string{"arg1", "arg2"}, + Secrets: map[string]string{"API_KEY": "secret-value"}, + }, + }, + } + jsonBody, err := json.Marshal(reqBody) + Expect(err).NotTo(HaveOccurred()) + + // Create a test request + req := httptest.NewRequest(http.MethodPut, "/v1/agents/mcp-update-agent?namespace=default", bytes.NewBuffer(jsonBody)) + req.Header.Set("Content-Type", "application/json") + recorder = httptest.NewRecorder() + + // Serve the request + router.ServeHTTP(recorder, req) + + // Verify the response + Expect(recorder.Code).To(Equal(http.StatusOK)) + + // Parse the response + var response AgentResponse + err = json.Unmarshal(recorder.Body.Bytes(), &response) + Expect(err).NotTo(HaveOccurred()) + + // Verify response has updated values + Expect(response.SystemPrompt).To(Equal("Updated prompt")) + Expect(response.MCPServers).To(HaveKey("new")) + Expect(response.MCPServers).To(HaveKey("existing")) + Expect(response.MCPServers).NotTo(HaveKey("old")) + + // Verify agent was updated in Kubernetes with new MCP servers + var updatedAgent acp.Agent + Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: "mcp-update-agent", + Namespace: "default", + }, &updatedAgent)).To(Succeed()) + + Expect(updatedAgent.Spec.System).To(Equal("Updated prompt")) + Expect(len(updatedAgent.Spec.MCPServers)).To(Equal(2)) + + // Extract MCP server names from the updated agent + mcpServerNames := []string{} + for _, mcpRef := range updatedAgent.Spec.MCPServers { + mcpServerNames = append(mcpServerNames, mcpRef.Name) + } + + // Verify both new MCP servers exist in the agent's references + Expect(mcpServerNames).To(ContainElement("mcp-update-agent-new")) + Expect(mcpServerNames).To(ContainElement("mcp-update-agent-existing")) + + // Old MCP server should no longer be in the agent's references + Expect(mcpServerNames).NotTo(ContainElement("mcp-update-agent-old")) + + // Verify new MCP server was created + var newMCP acp.MCPServer + Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: "mcp-update-agent-new", + Namespace: "default", + }, &newMCP)).To(Succeed()) + Expect(newMCP.Spec.Transport).To(Equal("http")) + Expect(newMCP.Spec.URL).To(Equal("http://newserver.com")) + + // Verify MCP server with secrets + var existingMCP acp.MCPServer + Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: "mcp-update-agent-existing", + Namespace: "default", + }, &existingMCP)).To(Succeed()) + Expect(existingMCP.Spec.Transport).To(Equal("stdio")) + Expect(existingMCP.Spec.Command).To(Equal("updated-cmd")) + Expect(existingMCP.Spec.Args).To(ConsistOf("arg1", "arg2")) + + // Verify the secret was created + var secret corev1.Secret + Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: "mcp-update-agent-existing-secrets", + Namespace: "default", + }, &secret)).To(Succeed()) + Expect(string(secret.Data["API_KEY"])).To(Equal("secret-value")) + + // Verify old MCP server was deleted + var oldMCPCheck acp.MCPServer + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: "mcp-update-agent-old", + Namespace: "default", + }, &oldMCPCheck) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }) + + It("should return 404 if agent doesn't exist", func() { + // Prepare update request + reqBody := UpdateAgentRequest{ + LLM: "non-existent-llm", + SystemPrompt: "Test prompt", + } + jsonBody, err := json.Marshal(reqBody) + Expect(err).NotTo(HaveOccurred()) + + // Create a test request for non-existent agent + req := httptest.NewRequest(http.MethodPut, "/v1/agents/non-existent-agent?namespace=default", bytes.NewBuffer(jsonBody)) + req.Header.Set("Content-Type", "application/json") + recorder = httptest.NewRecorder() + + // Serve the request + router.ServeHTTP(recorder, req) + + // Verify we get 404 Not Found + Expect(recorder.Code).To(Equal(http.StatusNotFound)) + var errorResponse map[string]string + err = json.Unmarshal(recorder.Body.Bytes(), &errorResponse) + Expect(err).NotTo(HaveOccurred()) + Expect(errorResponse["error"]).To(Equal("Agent not found")) + }) + + It("should return 404 if LLM doesn't exist", func() { + // Create an agent first + agent := &acp.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing-agent", + Namespace: "default", + }, + Spec: acp.AgentSpec{ + LLMRef: acp.LocalObjectReference{Name: "existing-llm"}, + System: "Existing prompt", + }, + } + Expect(k8sClient.Create(ctx, agent)).To(Succeed()) + + // Prepare update request with non-existent LLM + reqBody := UpdateAgentRequest{ + LLM: "non-existent-llm", + SystemPrompt: "Updated prompt", + } + jsonBody, err := json.Marshal(reqBody) + Expect(err).NotTo(HaveOccurred()) + + // Create a test request + req := httptest.NewRequest(http.MethodPut, "/v1/agents/existing-agent?namespace=default", bytes.NewBuffer(jsonBody)) + req.Header.Set("Content-Type", "application/json") + recorder = httptest.NewRecorder() + + // Serve the request + router.ServeHTTP(recorder, req) + + // Verify we get 404 Not Found for LLM + Expect(recorder.Code).To(Equal(http.StatusNotFound)) + var errorResponse map[string]string + err = json.Unmarshal(recorder.Body.Bytes(), &errorResponse) + Expect(err).NotTo(HaveOccurred()) + Expect(errorResponse["error"]).To(Equal("LLM not found")) + }) + + It("should return 400 for invalid MCP server configuration", func() { + // Create an LLM + createTestLLM("test-llm-invalid", "default") + + // Create an agent + agent := &acp.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-invalid-mcp", + Namespace: "default", + }, + Spec: acp.AgentSpec{ + LLMRef: acp.LocalObjectReference{Name: "test-llm-invalid"}, + System: "Original prompt", + }, + } + Expect(k8sClient.Create(ctx, agent)).To(Succeed()) + + // Prepare update request with invalid MCP server config (missing URL for http transport) + reqBody := UpdateAgentRequest{ + LLM: "test-llm-invalid", + SystemPrompt: "Updated prompt", + MCPServers: map[string]MCPServerConfig{ + "invalid": { + Transport: "http", + // URL is required for http transport but missing + }, + }, + } + jsonBody, err := json.Marshal(reqBody) + Expect(err).NotTo(HaveOccurred()) + + // Create a test request + req := httptest.NewRequest(http.MethodPut, "/v1/agents/agent-invalid-mcp?namespace=default", bytes.NewBuffer(jsonBody)) + req.Header.Set("Content-Type", "application/json") + recorder = httptest.NewRecorder() + + // Serve the request + router.ServeHTTP(recorder, req) + + // Verify we get 400 Bad Request + Expect(recorder.Code).To(Equal(http.StatusBadRequest)) + var errorResponse map[string]string + err = json.Unmarshal(recorder.Body.Bytes(), &errorResponse) + Expect(err).NotTo(HaveOccurred()) + Expect(errorResponse["error"]).To(ContainSubstring("Invalid MCP server config")) + Expect(errorResponse["error"]).To(ContainSubstring("url required")) + }) + + It("should require a namespace parameter", func() { + // Prepare update request + reqBody := UpdateAgentRequest{ + LLM: "test-llm", + SystemPrompt: "Test prompt", + } + jsonBody, err := json.Marshal(reqBody) + Expect(err).NotTo(HaveOccurred()) + + // Create a test request without namespace + req := httptest.NewRequest(http.MethodPut, "/v1/agents/some-agent", bytes.NewBuffer(jsonBody)) + req.Header.Set("Content-Type", "application/json") + recorder = httptest.NewRecorder() + + // Serve the request + router.ServeHTTP(recorder, req) + + // Verify we get 400 Bad Request + Expect(recorder.Code).To(Equal(http.StatusBadRequest)) + var errorResponse map[string]string + err = json.Unmarshal(recorder.Body.Bytes(), &errorResponse) + Expect(err).NotTo(HaveOccurred()) + Expect(errorResponse["error"]).To(Equal("namespace query parameter is required")) + }) + }) }) })