Migrate clients to default group (#1265)

Author: eleftherias

docs/server/docs.go

@@ -40,12 +40,12 @@ func TestGroupsRouter(t *testing.T) {
 			path:   "/",
 			setupMock: func(m *mocks.MockManager) {
 				m.EXPECT().List(gomock.Any()).Return([]*groups.Group{
-					{Name: "group1"},
-					{Name: "group2"},
+					{Name: "group1", RegisteredClients: []string{}},
+					{Name: "group2", RegisteredClients: []string{}},
 				}, nil)
 			},
 			expectedStatus: http.StatusOK,
-			expectedBody:   `{"groups":[{"name":"group1"},{"name":"group2"}]}`,
+			expectedBody:   `{"groups":[{"name":"group1", "registered_clients": []},{"name":"group2", "registered_clients": []}]}`,
 		},
 		{
 			name:   "list groups error",
@@ -106,10 +106,11 @@ func TestGroupsRouter(t *testing.T) {
 			method: "GET",
 			path:   "/testgroup",
 			setupMock: func(m *mocks.MockManager) {
-				m.EXPECT().Get(gomock.Any(), "testgroup").Return(&groups.Group{Name: "testgroup"}, nil)
+				m.EXPECT().Get(gomock.Any(), "testgroup").
+					Return(&groups.Group{Name: "testgroup", RegisteredClients: []string{}}, nil)
 			},
 			expectedStatus: http.StatusOK,
-			expectedBody:   `{"name":"testgroup"}`,
+			expectedBody:   `{"name":"testgroup", "registered_clients": []}`,
 		},
 		{
 			name:   "get group not found",

docs/server/swagger.json

@@ -13,7 +13,8 @@ const DefaultGroup = "default"
 
 // Group represents a logical grouping of MCP servers.
 type Group struct {
-	Name string `json:"name"`
+	Name              string   `json:"name"`
+	RegisteredClients []string `json:"registered_clients"`
 }
 
 // WriteJSON serializes the Group to JSON and writes it to the provided writer
@@ -52,4 +53,7 @@ type Manager interface {
 
 	// ListWorkloadsInGroup returns all workload names that belong to the specified group.
 	ListWorkloadsInGroup(ctx context.Context, groupName string) ([]string, error)
+
+	// RegisterClient registers a client with the group.
+	RegisterClient(ctx context.Context, groupName, clientName string) error
 }

docs/server/swagger.yaml

@@ -127,13 +127,13 @@ func TestManager_Get(t *testing.T) {
 			name:      "successful retrieval",
 			groupName: testGroupName,
 			setupMock: func(mock *mocks.MockStore) {
-				groupData := `{"name": "` + testGroupName + `"}`
+				groupData := `{"name": "` + testGroupName + `", "registered_clients": ["client1", "client2"]}`
 				mock.EXPECT().
 					GetReader(gomock.Any(), testGroupName).
 					Return(io.NopCloser(strings.NewReader(groupData)), nil)
 			},
 			expectError:   false,
-			expectedGroup: &Group{Name: testGroupName},
+			expectedGroup: &Group{Name: testGroupName, RegisteredClients: []string{"client1", "client2"}},
 		},
 		{
 			name:      "group not found",
@@ -183,6 +183,7 @@ func TestManager_Get(t *testing.T) {
 			} else {
 				assert.NoError(t, err)
 				assert.Equal(t, tt.expectedGroup.Name, group.Name)
+				assert.Equal(t, tt.expectedGroup.RegisteredClients, group.RegisteredClients)
 			}
 		})
 	}
@@ -503,6 +504,91 @@ func TestManager_GetWorkloadGroup(t *testing.T) {
 	}
 }
 
+// TestManager_RegisterClient tests client registration
+func TestManager_RegisterClient(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		groupName   string
+		clientName  string
+		setupMock   func(*mocks.MockStore)
+		expectError bool
+		errorMsg    string
+	}{
+		{
+			name:       "successful client registration",
+			groupName:  testGroupName,
+			clientName: "test-client",
+			setupMock: func(mock *mocks.MockStore) {
+				// First call to Get() - return existing group
+				groupData := `{"name": "` + testGroupName + `", "registered_clients": []}`
+				mock.EXPECT().
+					GetReader(gomock.Any(), testGroupName).
+					Return(io.NopCloser(strings.NewReader(groupData)), nil)
+
+				// Second call to saveGroup()
+				mock.EXPECT().
+					GetWriter(gomock.Any(), testGroupName).
+					Return(&mockWriteCloser{}, nil)
+			},
+			expectError: false,
+		},
+		{
+			name:       "client already registered",
+			groupName:  testGroupName,
+			clientName: "existing-client",
+			setupMock: func(mock *mocks.MockStore) {
+				// Return group with client already registered
+				groupData := `{"name": "` + testGroupName + `", "registered_clients": ["existing-client"]}`
+				mock.EXPECT().
+					GetReader(gomock.Any(), testGroupName).
+					Return(io.NopCloser(strings.NewReader(groupData)), nil)
+			},
+			expectError: true,
+			errorMsg:    "is already registered",
+		},
+		{
+			name:       "group not found",
+			groupName:  "nonexistent-group",
+			clientName: "test-client",
+			setupMock: func(mock *mocks.MockStore) {
+				mock.EXPECT().
+					GetReader(gomock.Any(), "nonexistent-group").
+					Return(nil, &os.PathError{Op: "open", Path: "nonexistent-group", Err: os.ErrNotExist})
+			},
+			expectError: true,
+			errorMsg:    "failed to get group",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			mockStore := mocks.NewMockStore(ctrl)
+			manager := &manager{groupStore: mockStore}
+
+			// Set up mock expectations
+			tt.setupMock(mockStore)
+
+			// Execute operation
+			err := manager.RegisterClient(context.Background(), tt.groupName, tt.clientName)
+
+			// Verify results
+			if tt.expectError {
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), tt.errorMsg)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
 // mockWriteCloser implements io.WriteCloser for testing
 type mockWriteCloser struct {
 	data []byte

pkg/api/v1/groups_test.go

@@ -49,7 +49,10 @@ func (m *manager) Create(ctx context.Context, name string) error {
 		return thverrors.NewGroupAlreadyExistsError(fmt.Sprintf("group '%s' already exists", name), nil)
 	}
 
-	group := &Group{Name: name}
+	group := &Group{
+		Name:              name,
+		RegisteredClients: []string{},
+	}
 	return m.saveGroup(ctx, group)
 }
 
@@ -150,6 +153,28 @@ func (m *manager) ListWorkloadsInGroup(ctx context.Context, groupName string) ([
 	return groupWorkloads, nil
 }
 
+// RegisterClient registers a client with the group
+func (m *manager) RegisterClient(ctx context.Context, groupName, clientName string) error {
+	// Get the existing group
+	group, err := m.Get(ctx, groupName)
+	if err != nil {
+		return fmt.Errorf("failed to get group %s: %w", groupName, err)
+	}
+
+	// Check if client is already registered
+	for _, existingClient := range group.RegisteredClients {
+		if existingClient == clientName {
+			return fmt.Errorf("client '%s' is already registered with group '%s'", clientName, groupName)
+		}
+	}
+
+	// Add the client to the group
+	group.RegisteredClients = append(group.RegisteredClients, clientName)
+
+	// Save the updated group
+	return m.saveGroup(ctx, group)
+}
+
 // saveGroup saves the group to the group state store
 func (m *manager) saveGroup(ctx context.Context, group *Group) error {
 	writer, err := m.groupStore.GetWriter(ctx, group.Name)

pkg/groups/group.go

@@ -88,6 +88,12 @@ func performDefaultGroupMigration() {
 		fmt.Println("No workloads needed migration to default group")
 	}
 
+	// Migrate client configurations from global config to default group
+	if err := migrateClientConfigs(context.Background(), groupManager); err != nil {
+		logger.Errorf("Failed to migrate client configurations: %v", err)
+		return
+	}
+
 	// Mark default group migration as completed
 	err = config.UpdateConfig(func(c *config.Config) {
 		c.DefaultGroupMigration = true
@@ -99,6 +105,64 @@ func performDefaultGroupMigration() {
 	}
 }
 
+// migrateClientConfigs migrates client configurations from global config to default group
+func migrateClientConfigs(ctx context.Context, groupManager Manager) error {
+	appConfig := config.GetConfig()
+
+	// If there are no registered clients, nothing to migrate
+	if len(appConfig.Clients.RegisteredClients) == 0 {
+		logger.Infof("No client configurations to migrate")
+		return nil
+	}
+
+	fmt.Printf("Migrating %d client configurations to default group...\n", len(appConfig.Clients.RegisteredClients))
+
+	// Get the default group
+	defaultGroup, err := groupManager.Get(ctx, DefaultGroupName)
+	if err != nil {
+		return fmt.Errorf("failed to get default group: %w", err)
+	}
+
+	migratedCount := 0
+	// Copy all registered clients to the default group
+	for _, clientName := range appConfig.Clients.RegisteredClients {
+		// Check if client is already in the group (avoid duplicates)
+		alreadyRegistered := false
+		for _, existingClient := range defaultGroup.RegisteredClients {
+			if existingClient == clientName {
+				alreadyRegistered = true
+				break
+			}
+		}
+
+		if !alreadyRegistered {
+			if err := groupManager.RegisterClient(ctx, DefaultGroupName, clientName); err != nil {
+				logger.Warnf("Failed to register client %s to default group: %v", clientName, err)
+				continue
+			}
+			migratedCount++
+		}
+	}
+
+	if migratedCount > 0 {
+		fmt.Printf("Successfully migrated %d client configurations to default group '%s'\n", migratedCount, DefaultGroupName)
+
+		// Clear the global client configurations after successful migration
+		err = config.UpdateConfig(func(c *config.Config) {
+			c.Clients.RegisteredClients = []string{}
+		})
+		if err != nil {
+			logger.Warnf("Failed to clear global client configurations after migration: %v", err)
+		} else {
+			logger.Infof("Cleared global client configurations")
+		}
+	} else {
+		logger.Infof("No client configurations needed migration")
+	}
+
+	return nil
+}
+
 // createDefaultGroup creates the default group if it doesn't exist
 func createDefaultGroup(ctx context.Context, groupManager Manager) error {
 	logger.Infof("Creating default group '%s'", DefaultGroupName)