From 72fef66f8e3c856fcd334d675573f3a3da5d3988 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 20:03:43 +0000 Subject: [PATCH 1/2] Initial plan From 75bc8595bd3abbcdd21fd62a85bc392c4903e0ba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 20:08:24 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=90=9B=20Add=20cluster=20group=20hand?= =?UTF-8?q?ler=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: GitHub --- .../handlers/workloads/cluster_groups_test.go | 300 ++++++++++++++++-- 1 file changed, 281 insertions(+), 19 deletions(-) diff --git a/pkg/api/handlers/workloads/cluster_groups_test.go b/pkg/api/handlers/workloads/cluster_groups_test.go index 09b974a91a..907e3c4ade 100644 --- a/pkg/api/handlers/workloads/cluster_groups_test.go +++ b/pkg/api/handlers/workloads/cluster_groups_test.go @@ -1,28 +1,290 @@ package workloads import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" "testing" + "github.com/kubestellar/console/pkg/models" + "github.com/kubestellar/console/pkg/test" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" ) -// Note: Most functions in cluster_groups.go are HTTP handlers that require -// complex setup (Fiber app, mock K8s client, mock store). We skip testing those -// and focus on any pure helper functions. - -// As of the current implementation, cluster_groups.go contains: -// - HTTP handler methods (ListClusterGroups, CreateClusterGroup, etc.) - require Fiber context -// - Persistence methods (persistClusterGroup, deletePersistedClusterGroup) - require store -// - Cache refresh methods (LoadPersistedClusterGroups, StartCacheRefresh, StopCacheRefresh) - require store and background goroutines -// -// All of these require complex integration test setup with mocks. -// Per the task requirements: "Test ONLY pure functions and helpers - do NOT test HTTP handlers that require complex server setup" -// -// Therefore, this test file serves as a placeholder to document that cluster_groups.go -// contains no testable pure functions without mocking infrastructure. - -func TestClusterGroupsPlaceholder(t *testing.T) { - // This test exists to provide minimal coverage and document the decision - // to skip testing cluster_groups.go due to lack of pure functions. - assert.NotEmpty(t, allHealthyClustersGroupName, "built-in group name should be defined") +func resetClusterGroups(t *testing.T) { + t.Helper() + clearGroups := func() { + clusterGroupsMu.Lock() + clusterGroups = make(map[string]ClusterGroup) + clusterGroupsMu.Unlock() + } + clearGroups() + t.Cleanup(clearGroups) +} + +func newJSONRequest(t *testing.T, method, path string, body any) *http.Request { + t.Helper() + data, err := json.Marshal(body) + require.NoError(t, err) + req := httptest.NewRequest(method, path, bytes.NewReader(data)) + req.Header.Set("Content-Type", "application/json") + return req +} + +func TestListClusterGroups_PrependsBuiltInGroup(t *testing.T) { + env := setupTestEnv(t) + resetClusterGroups(t) + + clusterGroupsMu.Lock() + clusterGroups["user-group"] = ClusterGroup{Name: "user-group", Kind: "static", Clusters: []string{"c1"}} + clusterGroupsMu.Unlock() + + h := NewWorkloadHandlers(nil, env.Hub, env.Store) + env.App.Get("/api/cluster-groups", h.ListClusterGroups) + + resp, err := env.App.Test(httptest.NewRequest(http.MethodGet, "/api/cluster-groups", nil), -1) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + + var payload struct { + Groups []ClusterGroup `json:"groups"` + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&payload)) + require.GreaterOrEqual(t, len(payload.Groups), 2) + assert.Equal(t, allHealthyClustersGroupName, payload.Groups[0].Name) + assert.True(t, payload.Groups[0].BuiltIn) + assert.Equal(t, []string{}, payload.Groups[0].Clusters) +} + +func TestCreateClusterGroup_ValidationAndPersistence(t *testing.T) { + tests := []struct { + name string + body any + wantStatus int + }{ + { + name: "empty name rejected", + body: ClusterGroup{Kind: "static", Clusters: []string{"c1"}}, + wantStatus: http.StatusBadRequest, + }, + { + name: "reserved name rejected", + body: ClusterGroup{Name: allHealthyClustersGroupName, Kind: "static", Clusters: []string{"c1"}}, + wantStatus: http.StatusBadRequest, + }, + { + name: "static group requires clusters", + body: ClusterGroup{Name: "no-clusters", Kind: "static", Clusters: []string{}}, + wantStatus: http.StatusBadRequest, + }, + { + name: "dynamic group may omit clusters", + body: ClusterGroup{Name: "dynamic-ok", Kind: "dynamic", Clusters: []string{}}, + wantStatus: http.StatusCreated, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + env := setupTestEnv(t) + resetClusterGroups(t) + h := NewWorkloadHandlers(nil, env.Hub, env.Store) + env.App.Post("/api/cluster-groups", h.CreateClusterGroup) + + resp, err := env.App.Test(newJSONRequest(t, http.MethodPost, "/api/cluster-groups", tt.body), -1) + require.NoError(t, err) + assert.Equal(t, tt.wantStatus, resp.StatusCode) + }) + } + + t.Run("persists created group", func(t *testing.T) { + env := setupTestEnv(t) + resetClusterGroups(t) + mockStore := env.Store.(*test.MockStore) + mockStore.ExpectedCalls = filterExpectedCalls(mockStore.ExpectedCalls, "SaveClusterGroup") + mockStore.On("SaveClusterGroup", "persist-me", mock.MatchedBy(func(data []byte) bool { + var group ClusterGroup + if err := json.Unmarshal(data, &group); err != nil { + return false + } + return group.Name == "persist-me" && group.Kind == "static" && len(group.Clusters) == 1 + })).Return(nil).Once() + + h := NewWorkloadHandlers(nil, env.Hub, env.Store) + env.App.Post("/api/cluster-groups", h.CreateClusterGroup) + + resp, err := env.App.Test(newJSONRequest(t, http.MethodPost, "/api/cluster-groups", ClusterGroup{ + Name: "persist-me", Kind: "static", Clusters: []string{"c1"}, + }), -1) + require.NoError(t, err) + assert.Equal(t, http.StatusCreated, resp.StatusCode) + mockStore.AssertExpectations(t) + }) +} + +func TestClusterGroupMutations_RequireAdmin(t *testing.T) { + tests := []struct { + name string + method string + path string + body []byte + }{ + { + name: "create forbidden for non-admin", + method: http.MethodPost, + path: "/api/cluster-groups", + body: mustMarshal(ClusterGroup{Name: "g1", Kind: "static", Clusters: []string{"c1"}}), + }, + { + name: "update forbidden for non-admin", + method: http.MethodPut, + path: "/api/cluster-groups/g1", + body: mustMarshal(ClusterGroup{Name: "g1", Kind: "static", Clusters: []string{"c1"}}), + }, + { + name: "delete forbidden for non-admin", + method: http.MethodDelete, + path: "/api/cluster-groups/g1", + }, + { + name: "sync forbidden for non-admin", + method: http.MethodPost, + path: "/api/cluster-groups/sync", + body: mustMarshal([]ClusterGroup{{Name: "g1", Kind: "static", Clusters: []string{"c1"}}}), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + env := setupTestEnv(t) + resetClusterGroups(t) + mockStore := env.Store.(*test.MockStore) + mockStore.ExpectedCalls = filterExpectedCalls(mockStore.ExpectedCalls, "GetUser") + mockStore.On("GetUser", testAdminUserID).Return(&models.User{ + ID: testAdminUserID, + Role: models.UserRoleViewer, + }, nil).Once() + + h := NewWorkloadHandlers(nil, env.Hub, env.Store) + env.App.Post("/api/cluster-groups", h.CreateClusterGroup) + env.App.Put("/api/cluster-groups/:name", h.UpdateClusterGroup) + env.App.Delete("/api/cluster-groups/:name", h.DeleteClusterGroup) + env.App.Post("/api/cluster-groups/sync", h.SyncClusterGroups) + + req := httptest.NewRequest(tt.method, tt.path, bytes.NewReader(tt.body)) + req.Header.Set("Content-Type", "application/json") + resp, err := env.App.Test(req, -1) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + mockStore.AssertExpectations(t) + }) + } +} + +func TestUpdateAndDeleteClusterGroup_Persistence(t *testing.T) { + t.Run("update persists URL name", func(t *testing.T) { + env := setupTestEnv(t) + resetClusterGroups(t) + mockStore := env.Store.(*test.MockStore) + mockStore.ExpectedCalls = filterExpectedCalls(mockStore.ExpectedCalls, "SaveClusterGroup") + mockStore.On("SaveClusterGroup", "group-from-url", mock.MatchedBy(func(data []byte) bool { + var group ClusterGroup + if err := json.Unmarshal(data, &group); err != nil { + return false + } + return group.Name == "group-from-url" && group.Color == "red" + })).Return(nil).Once() + + clusterGroupsMu.Lock() + clusterGroups["group-from-url"] = ClusterGroup{Name: "group-from-url", Kind: "static", Clusters: []string{"c1"}} + clusterGroupsMu.Unlock() + + h := NewWorkloadHandlers(nil, env.Hub, env.Store) + env.App.Put("/api/cluster-groups/:name", h.UpdateClusterGroup) + + resp, err := env.App.Test(newJSONRequest(t, http.MethodPut, "/api/cluster-groups/group-from-url", ClusterGroup{ + Name: "ignored-body-name", Kind: "static", Clusters: []string{"c1"}, Color: "red", + }), -1) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + mockStore.AssertExpectations(t) + }) + + t.Run("delete removes persisted group", func(t *testing.T) { + env := setupTestEnv(t) + resetClusterGroups(t) + mockStore := env.Store.(*test.MockStore) + mockStore.ExpectedCalls = filterExpectedCalls(mockStore.ExpectedCalls, "DeleteClusterGroup") + mockStore.On("DeleteClusterGroup", "to-delete").Return(nil).Once() + + clusterGroupsMu.Lock() + clusterGroups["to-delete"] = ClusterGroup{Name: "to-delete", Kind: "static", Clusters: []string{"c1"}} + clusterGroupsMu.Unlock() + + h := NewWorkloadHandlers(nil, env.Hub, env.Store) + env.App.Delete("/api/cluster-groups/:name", h.DeleteClusterGroup) + + resp, err := env.App.Test(httptest.NewRequest(http.MethodDelete, "/api/cluster-groups/to-delete", nil), -1) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + mockStore.AssertExpectations(t) + }) +} + +func TestSyncClusterGroups_BodySizeAndFiltering(t *testing.T) { + t.Run("empty body rejected", func(t *testing.T) { + env := setupTestEnv(t) + resetClusterGroups(t) + h := NewWorkloadHandlers(nil, env.Hub, env.Store) + env.App.Post("/api/cluster-groups/sync", h.SyncClusterGroups) + + req := httptest.NewRequest(http.MethodPost, "/api/cluster-groups/sync", http.NoBody) + req.Header.Set("Content-Type", "application/json") + resp, err := env.App.Test(req, -1) + require.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + }) + + t.Run("oversized body rejected", func(t *testing.T) { + env := setupTestEnv(t) + resetClusterGroups(t) + h := NewWorkloadHandlers(nil, env.Hub, env.Store) + env.App.Post("/api/cluster-groups/sync", h.SyncClusterGroups) + + oversized := bytes.Repeat([]byte("a"), (1<<20)+1) + req := httptest.NewRequest(http.MethodPost, "/api/cluster-groups/sync", bytes.NewReader(oversized)) + req.Header.Set("Content-Type", "application/json") + resp, err := env.App.Test(req, -1) + require.NoError(t, err) + assert.Equal(t, http.StatusRequestEntityTooLarge, resp.StatusCode) + }) + + t.Run("reserved name is filtered during sync", func(t *testing.T) { + env := setupTestEnv(t) + resetClusterGroups(t) + h := NewWorkloadHandlers(nil, env.Hub, env.Store) + env.App.Post("/api/cluster-groups/sync", h.SyncClusterGroups) + + resp, err := env.App.Test(newJSONRequest(t, http.MethodPost, "/api/cluster-groups/sync", []ClusterGroup{ + {Name: allHealthyClustersGroupName, Kind: "dynamic", Clusters: []string{"c1"}}, + {Name: "kept", Kind: "static", Clusters: []string{"c2"}}, + }), -1) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Contains(t, string(body), `"synced":1`) + + clusterGroupsMu.RLock() + _, reservedExists := clusterGroups[allHealthyClustersGroupName] + _, keptExists := clusterGroups["kept"] + clusterGroupsMu.RUnlock() + assert.False(t, reservedExists) + assert.True(t, keptExists) + }) }