diff --git a/api/gen/proto/go/teleport/machineid/v1/bot_instance_service.pb.go b/api/gen/proto/go/teleport/machineid/v1/bot_instance_service.pb.go index 3bef9f97a0f17..71465c318a82b 100644 --- a/api/gen/proto/go/teleport/machineid/v1/bot_instance_service.pb.go +++ b/api/gen/proto/go/teleport/machineid/v1/bot_instance_service.pb.go @@ -21,6 +21,7 @@ package machineidv1 import ( + types "github.com/gravitational/teleport/api/types" protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoimpl "google.golang.org/protobuf/runtime/protoimpl" emptypb "google.golang.org/protobuf/types/known/emptypb" @@ -108,8 +109,10 @@ type ListBotInstancesRequest struct { PageToken string `protobuf:"bytes,3,opt,name=page_token,json=pageToken,proto3" json:"page_token,omitempty"` // A search term used to filter the results. If non-empty, it's used to match against supported fields. FilterSearchTerm string `protobuf:"bytes,4,opt,name=filter_search_term,json=filterSearchTerm,proto3" json:"filter_search_term,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // The sort config to use for the results. If empty, the default sort field and order is used. + Sort *types.SortBy `protobuf:"bytes,5,opt,name=sort,proto3" json:"sort,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *ListBotInstancesRequest) Reset() { @@ -170,6 +173,13 @@ func (x *ListBotInstancesRequest) GetFilterSearchTerm() string { return "" } +func (x *ListBotInstancesRequest) GetSort() *types.SortBy { + if x != nil { + return x.Sort + } + return nil +} + // Response for ListBotInstances. type ListBotInstancesResponse struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -368,17 +378,18 @@ var File_teleport_machineid_v1_bot_instance_service_proto protoreflect.FileDescr const file_teleport_machineid_v1_bot_instance_service_proto_rawDesc = "" + "\n" + - "0teleport/machineid/v1/bot_instance_service.proto\x12\x15teleport.machineid.v1\x1a\x1bgoogle/protobuf/empty.proto\x1a(teleport/machineid/v1/bot_instance.proto\"S\n" + + "0teleport/machineid/v1/bot_instance_service.proto\x12\x15teleport.machineid.v1\x1a\x1bgoogle/protobuf/empty.proto\x1a!teleport/legacy/types/types.proto\x1a(teleport/machineid/v1/bot_instance.proto\"S\n" + "\x15GetBotInstanceRequest\x12\x19\n" + "\bbot_name\x18\x01 \x01(\tR\abotName\x12\x1f\n" + "\vinstance_id\x18\x02 \x01(\tR\n" + - "instanceId\"\xab\x01\n" + + "instanceId\"\xce\x01\n" + "\x17ListBotInstancesRequest\x12&\n" + "\x0ffilter_bot_name\x18\x01 \x01(\tR\rfilterBotName\x12\x1b\n" + "\tpage_size\x18\x02 \x01(\x05R\bpageSize\x12\x1d\n" + "\n" + "page_token\x18\x03 \x01(\tR\tpageToken\x12,\n" + - "\x12filter_search_term\x18\x04 \x01(\tR\x10filterSearchTerm\"\x8b\x01\n" + + "\x12filter_search_term\x18\x04 \x01(\tR\x10filterSearchTerm\x12!\n" + + "\x04sort\x18\x05 \x01(\v2\r.types.SortByR\x04sort\"\x8b\x01\n" + "\x18ListBotInstancesResponse\x12G\n" + "\rbot_instances\x18\x01 \x03(\v2\".teleport.machineid.v1.BotInstanceR\fbotInstances\x12&\n" + "\x0fnext_page_token\x18\x02 \x01(\tR\rnextPageToken\"V\n" + @@ -415,26 +426,28 @@ var file_teleport_machineid_v1_bot_instance_service_proto_goTypes = []any{ (*DeleteBotInstanceRequest)(nil), // 3: teleport.machineid.v1.DeleteBotInstanceRequest (*SubmitHeartbeatRequest)(nil), // 4: teleport.machineid.v1.SubmitHeartbeatRequest (*SubmitHeartbeatResponse)(nil), // 5: teleport.machineid.v1.SubmitHeartbeatResponse - (*BotInstance)(nil), // 6: teleport.machineid.v1.BotInstance - (*BotInstanceStatusHeartbeat)(nil), // 7: teleport.machineid.v1.BotInstanceStatusHeartbeat - (*emptypb.Empty)(nil), // 8: google.protobuf.Empty + (*types.SortBy)(nil), // 6: types.SortBy + (*BotInstance)(nil), // 7: teleport.machineid.v1.BotInstance + (*BotInstanceStatusHeartbeat)(nil), // 8: teleport.machineid.v1.BotInstanceStatusHeartbeat + (*emptypb.Empty)(nil), // 9: google.protobuf.Empty } var file_teleport_machineid_v1_bot_instance_service_proto_depIdxs = []int32{ - 6, // 0: teleport.machineid.v1.ListBotInstancesResponse.bot_instances:type_name -> teleport.machineid.v1.BotInstance - 7, // 1: teleport.machineid.v1.SubmitHeartbeatRequest.heartbeat:type_name -> teleport.machineid.v1.BotInstanceStatusHeartbeat - 0, // 2: teleport.machineid.v1.BotInstanceService.GetBotInstance:input_type -> teleport.machineid.v1.GetBotInstanceRequest - 1, // 3: teleport.machineid.v1.BotInstanceService.ListBotInstances:input_type -> teleport.machineid.v1.ListBotInstancesRequest - 3, // 4: teleport.machineid.v1.BotInstanceService.DeleteBotInstance:input_type -> teleport.machineid.v1.DeleteBotInstanceRequest - 4, // 5: teleport.machineid.v1.BotInstanceService.SubmitHeartbeat:input_type -> teleport.machineid.v1.SubmitHeartbeatRequest - 6, // 6: teleport.machineid.v1.BotInstanceService.GetBotInstance:output_type -> teleport.machineid.v1.BotInstance - 2, // 7: teleport.machineid.v1.BotInstanceService.ListBotInstances:output_type -> teleport.machineid.v1.ListBotInstancesResponse - 8, // 8: teleport.machineid.v1.BotInstanceService.DeleteBotInstance:output_type -> google.protobuf.Empty - 5, // 9: teleport.machineid.v1.BotInstanceService.SubmitHeartbeat:output_type -> teleport.machineid.v1.SubmitHeartbeatResponse - 6, // [6:10] is the sub-list for method output_type - 2, // [2:6] is the sub-list for method input_type - 2, // [2:2] is the sub-list for extension type_name - 2, // [2:2] is the sub-list for extension extendee - 0, // [0:2] is the sub-list for field type_name + 6, // 0: teleport.machineid.v1.ListBotInstancesRequest.sort:type_name -> types.SortBy + 7, // 1: teleport.machineid.v1.ListBotInstancesResponse.bot_instances:type_name -> teleport.machineid.v1.BotInstance + 8, // 2: teleport.machineid.v1.SubmitHeartbeatRequest.heartbeat:type_name -> teleport.machineid.v1.BotInstanceStatusHeartbeat + 0, // 3: teleport.machineid.v1.BotInstanceService.GetBotInstance:input_type -> teleport.machineid.v1.GetBotInstanceRequest + 1, // 4: teleport.machineid.v1.BotInstanceService.ListBotInstances:input_type -> teleport.machineid.v1.ListBotInstancesRequest + 3, // 5: teleport.machineid.v1.BotInstanceService.DeleteBotInstance:input_type -> teleport.machineid.v1.DeleteBotInstanceRequest + 4, // 6: teleport.machineid.v1.BotInstanceService.SubmitHeartbeat:input_type -> teleport.machineid.v1.SubmitHeartbeatRequest + 7, // 7: teleport.machineid.v1.BotInstanceService.GetBotInstance:output_type -> teleport.machineid.v1.BotInstance + 2, // 8: teleport.machineid.v1.BotInstanceService.ListBotInstances:output_type -> teleport.machineid.v1.ListBotInstancesResponse + 9, // 9: teleport.machineid.v1.BotInstanceService.DeleteBotInstance:output_type -> google.protobuf.Empty + 5, // 10: teleport.machineid.v1.BotInstanceService.SubmitHeartbeat:output_type -> teleport.machineid.v1.SubmitHeartbeatResponse + 7, // [7:11] is the sub-list for method output_type + 3, // [3:7] is the sub-list for method input_type + 3, // [3:3] is the sub-list for extension type_name + 3, // [3:3] is the sub-list for extension extendee + 0, // [0:3] is the sub-list for field type_name } func init() { file_teleport_machineid_v1_bot_instance_service_proto_init() } diff --git a/api/proto/teleport/machineid/v1/bot_instance_service.proto b/api/proto/teleport/machineid/v1/bot_instance_service.proto index 8f1d2fb2fefb7..f0802486fccd4 100644 --- a/api/proto/teleport/machineid/v1/bot_instance_service.proto +++ b/api/proto/teleport/machineid/v1/bot_instance_service.proto @@ -17,6 +17,7 @@ syntax = "proto3"; package teleport.machineid.v1; import "google/protobuf/empty.proto"; +import "teleport/legacy/types/types.proto"; import "teleport/machineid/v1/bot_instance.proto"; option go_package = "github.com/gravitational/teleport/api/gen/proto/go/teleport/machineid/v1;machineidv1"; @@ -45,6 +46,8 @@ message ListBotInstancesRequest { string page_token = 3; // A search term used to filter the results. If non-empty, it's used to match against supported fields. string filter_search_term = 4; + // The sort config to use for the results. If empty, the default sort field and order is used. + types.SortBy sort = 5; } // Response for ListBotInstances. diff --git a/lib/auth/authclient/api.go b/lib/auth/authclient/api.go index 3631f66751fb4..f3a369ad38fe6 100644 --- a/lib/auth/authclient/api.go +++ b/lib/auth/authclient/api.go @@ -1231,7 +1231,7 @@ type Cache interface { GetBotInstance(ctx context.Context, botName, instanceID string) (*machineidv1.BotInstance, error) // ListBotInstances returns a page of BotInstance resources. - ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string) ([]*machineidv1.BotInstance, string, error) + ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy) ([]*machineidv1.BotInstance, string, error) } type NodeWrapper struct { diff --git a/lib/auth/machineid/machineidv1/bot_instance_service.go b/lib/auth/machineid/machineidv1/bot_instance_service.go index ff1ccf7454e46..24d59081885e2 100644 --- a/lib/auth/machineid/machineidv1/bot_instance_service.go +++ b/lib/auth/machineid/machineidv1/bot_instance_service.go @@ -54,7 +54,7 @@ type BotInstancesCache interface { GetBotInstance(ctx context.Context, botName, instanceID string) (*pb.BotInstance, error) // ListBotInstances returns a page of BotInstance resources. - ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string) ([]*pb.BotInstance, string, error) + ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy) ([]*pb.BotInstance, string, error) } // BotInstanceServiceConfig holds configuration options for the BotInstance gRPC @@ -157,7 +157,7 @@ func (b *BotInstanceService) ListBotInstances(ctx context.Context, req *pb.ListB return nil, trace.Wrap(err) } - res, nextToken, err := b.cache.ListBotInstances(ctx, req.FilterBotName, int(req.PageSize), req.PageToken, req.FilterSearchTerm) + res, nextToken, err := b.cache.ListBotInstances(ctx, req.FilterBotName, int(req.PageSize), req.PageToken, req.FilterSearchTerm, req.Sort) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/cache/bot_instance.go b/lib/cache/bot_instance.go index 06ba0a69b8d01..85fcf6d0764ce 100644 --- a/lib/cache/bot_instance.go +++ b/lib/cache/bot_instance.go @@ -20,6 +20,7 @@ import ( "context" "slices" "strings" + "time" "github.com/gravitational/trace" "google.golang.org/protobuf/proto" @@ -34,7 +35,8 @@ import ( type botInstanceIndex string const ( - botInstanceNameIndex botInstanceIndex = "name" + botInstanceNameIndex botInstanceIndex = "name" + botInstanceActiveAtIndex botInstanceIndex = "active_at_latest" ) func keyForNameIndex(botInstance *machineidv1.BotInstance) string { @@ -48,6 +50,22 @@ func makeNameIndexKey(botName string, instanceID string) string { return botName + "/" + instanceID } +func keyForActiveAtIndex(botInstance *machineidv1.BotInstance) string { + var recordedAt time.Time + + initialHeartbeatTime := botInstance.GetStatus().GetInitialHeartbeat().GetRecordedAt() + if initialHeartbeatTime != nil { + recordedAt = initialHeartbeatTime.AsTime() + } + + latestHeartbeats := botInstance.GetStatus().GetLatestHeartbeats() + if len(latestHeartbeats) > 0 { + recordedAt = latestHeartbeats[len(latestHeartbeats)-1].GetRecordedAt().AsTime() + } + + return recordedAt.Format(time.RFC3339) + "/" + botInstance.GetMetadata().GetName() +} + func newBotInstanceCollection(upstream services.BotInstance, w types.WatchKind) (*collection[*machineidv1.BotInstance, botInstanceIndex], error) { if upstream == nil { return nil, trace.BadParameter("missing parameter upstream (BotInstance)") @@ -60,12 +78,14 @@ func newBotInstanceCollection(upstream services.BotInstance, w types.WatchKind) map[botInstanceIndex]func(*machineidv1.BotInstance) string{ // Index on a combination of bot name and instance name botInstanceNameIndex: keyForNameIndex, + // Index on a combination of most recent heartbeat time and instance name + botInstanceActiveAtIndex: keyForActiveAtIndex, }), fetcher: func(ctx context.Context, loadSecrets bool) ([]*machineidv1.BotInstance, error) { var out []*machineidv1.BotInstance clientutils.IterateResources(ctx, func(ctx context.Context, limit int, start string) ([]*machineidv1.BotInstance, string, error) { - return upstream.ListBotInstances(ctx, "", limit, start, "") + return upstream.ListBotInstances(ctx, "", limit, start, "", nil) }, func(hcc *machineidv1.BotInstance) error { out = append(out, hcc) @@ -97,23 +117,42 @@ func (c *Cache) GetBotInstance(ctx context.Context, botName, instanceID string) } // ListBotInstances returns a page of BotInstance resources. -func (c *Cache) ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string) ([]*machineidv1.BotInstance, string, error) { +func (c *Cache) ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy) ([]*machineidv1.BotInstance, string, error) { ctx, span := c.Tracer.Start(ctx, "cache/ListBotInstances") defer span.End() + index := botInstanceNameIndex + keyFn := keyForNameIndex + var isDesc bool + if sort != nil { + isDesc = sort.IsDesc + + switch sort.Field { + case "bot_name": + index = botInstanceNameIndex + keyFn = keyForNameIndex + case "active_at_latest": + index = botInstanceActiveAtIndex + keyFn = keyForActiveAtIndex + default: + return nil, "", trace.BadParameter("unsupported sort %q but expected bot_name or active_at_latest", sort.Field) + } + } + lister := genericLister[*machineidv1.BotInstance, botInstanceIndex]{ cache: c, collection: c.collections.botInstances, - index: botInstanceNameIndex, + index: index, + isDesc: isDesc, defaultPageSize: defaults.DefaultChunkSize, upstreamList: func(ctx context.Context, limit int, start string) ([]*machineidv1.BotInstance, string, error) { - return c.Config.BotInstanceService.ListBotInstances(ctx, botName, limit, start, search) + return c.Config.BotInstanceService.ListBotInstances(ctx, botName, limit, start, search, sort) }, filter: func(b *machineidv1.BotInstance) bool { return matchBotInstance(b, botName, search) }, nextToken: func(b *machineidv1.BotInstance) string { - return keyForNameIndex(b) + return keyFn(b) }, } out, next, err := lister.list(ctx, diff --git a/lib/cache/bot_instance_test.go b/lib/cache/bot_instance_test.go index 6395eed93f104..0503476b3fb2b 100644 --- a/lib/cache/bot_instance_test.go +++ b/lib/cache/bot_instance_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/timestamppb" headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1" machineidv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/machineid/v1" @@ -55,7 +56,7 @@ func TestBotInstanceCache(t *testing.T) { return p.cache.GetBotInstance(ctx, "bot-1", key) }, cacheList: func(ctx context.Context) ([]*machineidv1.BotInstance, error) { - results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "") + results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil) return results, err }, create: func(ctx context.Context, resource *machineidv1.BotInstance) error { @@ -63,7 +64,7 @@ func TestBotInstanceCache(t *testing.T) { return err }, list: func(ctx context.Context) ([]*machineidv1.BotInstance, error) { - results, _, err := p.botInstanceService.ListBotInstances(ctx, "", 0, "", "") + results, _, err := p.botInstanceService.ListBotInstances(ctx, "", 0, "", "", nil) return results, err }, update: func(ctx context.Context, bi *machineidv1.BotInstance) error { @@ -106,12 +107,13 @@ func TestBotInstanceCachePaging(t *testing.T) { // Let the cache catch up require.EventuallyWithT(t, func(t *assert.CollectT) { - _, err := p.cache.GetBotInstance(ctx, "bot-1", "instance-2") + results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil) require.NoError(t, err) - }, 2*time.Second, 10*time.Millisecond) + require.Len(t, results, 5) + }, 10*time.Second, 100*time.Millisecond) // page size equal to total items - results, nextPageToken, err := p.cache.ListBotInstances(ctx, "", 0, "", "") + results, nextPageToken, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil) require.NoError(t, err) require.Empty(t, nextPageToken) require.Len(t, results, 5) @@ -122,7 +124,7 @@ func TestBotInstanceCachePaging(t *testing.T) { require.Equal(t, "instance-5", results[4].GetMetadata().GetName()) // page size smaller than total items - results, nextPageToken, err = p.cache.ListBotInstances(ctx, "", 3, "", "") + results, nextPageToken, err = p.cache.ListBotInstances(ctx, "", 3, "", "", nil) require.NoError(t, err) require.Equal(t, "bot-1/instance-4", nextPageToken) require.Len(t, results, 3) @@ -131,7 +133,7 @@ func TestBotInstanceCachePaging(t *testing.T) { require.Equal(t, "instance-3", results[2].GetMetadata().GetName()) // next page - results, nextPageToken, err = p.cache.ListBotInstances(ctx, "", 3, nextPageToken, "") + results, nextPageToken, err = p.cache.ListBotInstances(ctx, "", 3, nextPageToken, "", nil) require.NoError(t, err) require.Empty(t, nextPageToken) require.Len(t, results, 2) @@ -148,34 +150,33 @@ func TestBotInstanceCacheBotFilter(t *testing.T) { p := newTestPack(t, ForAuth) t.Cleanup(p.Close) - for n := range 2 { - for m := range 5 { - _, err := p.botInstanceService.CreateBotInstance(ctx, &machineidv1.BotInstance{ - Kind: types.KindBotInstance, - Version: types.V1, - Metadata: &headerv1.Metadata{}, - Spec: &machineidv1.BotInstanceSpec{ - BotName: "bot-" + strconv.Itoa(n+1), - InstanceId: "instance-" + strconv.Itoa((n+1)*(m+1)), - }, - Status: &machineidv1.BotInstanceStatus{}, - }) - require.NoError(t, err) - } + for n := range 10 { + _, err := p.botInstanceService.CreateBotInstance(ctx, &machineidv1.BotInstance{ + Kind: types.KindBotInstance, + Version: types.V1, + Metadata: &headerv1.Metadata{}, + Spec: &machineidv1.BotInstanceSpec{ + BotName: "bot-" + strconv.Itoa(n%2), + InstanceId: "instance-" + strconv.Itoa(n), + }, + Status: &machineidv1.BotInstanceStatus{}, + }) + require.NoError(t, err) } // Let the cache catch up require.EventuallyWithT(t, func(t *assert.CollectT) { - _, err := p.cache.GetBotInstance(ctx, "bot-2", "instance-10") + results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil) require.NoError(t, err) - }, 2*time.Second, 10*time.Millisecond) + require.Len(t, results, 10) + }, 10*time.Second, 100*time.Millisecond) - results, _, err := p.cache.ListBotInstances(ctx, "bot-2", 0, "", "") + results, _, err := p.cache.ListBotInstances(ctx, "bot-1", 0, "", "", nil) require.NoError(t, err) require.Len(t, results, 5) for _, b := range results { - require.Equal(t, "bot-2", b.GetSpec().GetBotName()) + require.Equal(t, "bot-1", b.GetSpec().GetBotName()) } } @@ -212,11 +213,161 @@ func TestBotInstanceCacheSearchFilter(t *testing.T) { // Let the cache catch up require.EventuallyWithT(t, func(t *assert.CollectT) { - _, err := p.cache.GetBotInstance(ctx, "bot-1", "instance-10") + results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil) require.NoError(t, err) - }, 2*time.Second, 10*time.Millisecond) + require.Len(t, results, 10) + }, 10*time.Second, 100*time.Millisecond) - results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "host-1") + results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "host-1", nil) require.NoError(t, err) require.Len(t, results, 5) } + +// TestBotInstanceCacheSorting tests that cache items are sorted. +func TestBotInstanceCacheSorting(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + p := newTestPack(t, ForAuth) + t.Cleanup(p.Close) + + items := []struct { + botName string + instanceId string + recordedAtSeconds int64 + }{ + {"bot-1", "instance-1", 2}, + {"bot-1", "instance-3", 1}, + {"bot-2", "instance-2", 3}, + } + + for _, b := range items { + instance := &machineidv1.BotInstance{ + Kind: types.KindBotInstance, + Version: types.V1, + Metadata: &headerv1.Metadata{}, + Spec: &machineidv1.BotInstanceSpec{ + BotName: b.botName, + InstanceId: b.instanceId, + }, + Status: &machineidv1.BotInstanceStatus{ + LatestHeartbeats: []*machineidv1.BotInstanceStatusHeartbeat{ + { + RecordedAt: ×tamppb.Timestamp{ + Seconds: b.recordedAtSeconds, + }, + }, + }, + }, + } + + _, err := p.botInstanceService.CreateBotInstance(ctx, instance) + require.NoError(t, err) + } + + // Let the cache catch up + require.EventuallyWithT(t, func(t *assert.CollectT) { + results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil) + require.NoError(t, err) + require.Len(t, results, 3) + }, 10*time.Second, 100*time.Millisecond) + + // sort ascending by active_at_latest + results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{ + Field: "active_at_latest", + IsDesc: false, + }) + require.NoError(t, err) + require.Len(t, results, 3) + require.Equal(t, "instance-3", results[0].GetMetadata().GetName()) + require.Equal(t, "instance-1", results[1].GetMetadata().GetName()) + require.Equal(t, "instance-2", results[2].GetMetadata().GetName()) + + // sort descending by active_at_latest + results, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{ + Field: "active_at_latest", + IsDesc: true, + }) + require.NoError(t, err) + require.Len(t, results, 3) + require.Equal(t, "instance-2", results[0].GetMetadata().GetName()) + require.Equal(t, "instance-1", results[1].GetMetadata().GetName()) + require.Equal(t, "instance-3", results[2].GetMetadata().GetName()) + + // sort ascending by bot_name + results, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", nil) // empty sort should default to `bot_name:asc` + require.NoError(t, err) + require.Len(t, results, 3) + require.Equal(t, "instance-1", results[0].GetMetadata().GetName()) + require.Equal(t, "instance-3", results[1].GetMetadata().GetName()) + require.Equal(t, "instance-2", results[2].GetMetadata().GetName()) + + // sort descending by bot_name + results, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{ + Field: "bot_name", + IsDesc: true, + }) + require.NoError(t, err) + require.Len(t, results, 3) + require.Equal(t, "instance-2", results[0].GetMetadata().GetName()) + require.Equal(t, "instance-3", results[1].GetMetadata().GetName()) + require.Equal(t, "instance-1", results[2].GetMetadata().GetName()) +} + +// TestBotInstanceCacheFallback tests that requests fallback to the upstream when the cache is unhealthy. +func TestBotInstanceCacheFallback(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + p := newTestPack(t, func(cfg Config) Config { + cfg.neverOK = true // Force the cache into an unhealthy state + return ForAuth(cfg) + }) + t.Cleanup(p.Close) + + _, err := p.botInstanceService.CreateBotInstance(ctx, &machineidv1.BotInstance{ + Kind: types.KindBotInstance, + Version: types.V1, + Metadata: &headerv1.Metadata{}, + Spec: &machineidv1.BotInstanceSpec{ + BotName: "bot-1", + InstanceId: "instance-1", + }, + Status: &machineidv1.BotInstanceStatus{}, + }) + require.NoError(t, err) + + // Let the cache catch up + require.EventuallyWithT(t, func(t *assert.CollectT) { + results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", nil) + require.NoError(t, err) + require.Len(t, results, 1) + }, 10*time.Second, 100*time.Millisecond) + + // sort ascending by bot_name + results, _, err := p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{ + Field: "bot_name", + IsDesc: false, + }) + require.NoError(t, err) // asc by bot_name is the only sort supported by the upstream + require.Len(t, results, 1) + + // sort descending by bot_name + _, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{ + Field: "bot_name", + IsDesc: true, + }) + require.Error(t, err) + require.Equal(t, "unsupported sort, only bot_name:asc is supported, but got \"bot_name\" (desc = true)", err.Error()) + + // sort ascending by active_at_latest + _, _, err = p.cache.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{ + Field: "active_at_latest", + IsDesc: false, + }) + require.Error(t, err) + require.Equal(t, "unsupported sort, only bot_name:asc is supported, but got \"active_at_latest\" (desc = false)", err.Error()) + +} diff --git a/lib/cache/generic_operations.go b/lib/cache/generic_operations.go index deb77e4a4400b..082d4724fc6e4 100644 --- a/lib/cache/generic_operations.go +++ b/lib/cache/generic_operations.go @@ -70,6 +70,8 @@ type genericLister[T any, I comparable] struct { collection *collection[T, I] // index of the collection to read with. index I + // isDesc indicates whether the lister should retrieve items in descending order. + isDesc bool // defaultPageSize optionally defines a page size to use if // one is not specified by the caller. If not set then // [defaults.DefaultChunkSize] is used. @@ -109,8 +111,13 @@ func (l genericLister[T, I]) listRange(ctx context.Context, pageSize int, startT pageSize = defaultPageSize } + fetchFn := rg.store.cache.Ascend + if l.isDesc { + fetchFn = rg.store.cache.Descend + } + var out []T - for sf := range rg.store.resources(l.index, startToken, endToken) { + for sf := range fetchFn(l.index, startToken, endToken) { if len(out) == pageSize { return out, l.nextToken(sf), nil } diff --git a/lib/services/bot_instance.go b/lib/services/bot_instance.go index cf6d6b9360ecf..50beac8a76472 100644 --- a/lib/services/bot_instance.go +++ b/lib/services/bot_instance.go @@ -22,6 +22,7 @@ import ( "github.com/gravitational/trace" machineidv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/machineid/v1" + "github.com/gravitational/teleport/api/types" ) // BotInstance is an interface for the BotInstance service. @@ -33,7 +34,7 @@ type BotInstance interface { GetBotInstance(ctx context.Context, botName, instanceID string) (*machineidv1.BotInstance, error) // ListBotInstances - ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string) ([]*machineidv1.BotInstance, string, error) + ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy) ([]*machineidv1.BotInstance, string, error) // DeleteBotInstance DeleteBotInstance(ctx context.Context, botName, instanceID string) error diff --git a/lib/services/local/bot_instance.go b/lib/services/local/bot_instance.go index bc568abb37ded..82090ea0f8b53 100644 --- a/lib/services/local/bot_instance.go +++ b/lib/services/local/bot_instance.go @@ -94,8 +94,13 @@ func (b *BotInstanceService) GetBotInstance(ctx context.Context, botName, instan // ListBotInstances lists all matching bot instances. A bot name and/or search terms can be optionally provided. // If an non-empty bot name is provided, only instances for that bot will be fetched. // If an non-empty search term is provided, only instances with a value containing the term in supported fields are fetched. -// Supported search fields include; bot name, instance id, hostname (latest), tbot version (latest), join method (latest) -func (b *BotInstanceService) ListBotInstances(ctx context.Context, botName string, pageSize int, lastKey string, search string) ([]*machineidv1.BotInstance, string, error) { +// Supported search fields include; bot name, instance id, hostname (latest), tbot version (latest), join method (latest). +// Sorting by bot name in ascending order is supported - an error is returned for any other sort type. +func (b *BotInstanceService) ListBotInstances(ctx context.Context, botName string, pageSize int, lastKey string, search string, sort *types.SortBy) ([]*machineidv1.BotInstance, string, error) { + if sort != nil && (sort.Field != "bot_name" || sort.IsDesc != false) { + return nil, "", trace.BadParameter("unsupported sort, only bot_name:asc is supported, but got %q (desc = %t)", sort.Field, sort.IsDesc) + } + var service *generic.ServiceWrapper[*machineidv1.BotInstance] if botName == "" { // If botName is empty, return instances for all bots by not using a service prefix diff --git a/lib/services/local/bot_instance_test.go b/lib/services/local/bot_instance_test.go index dc9a451faba17..7d0b4e49f8f26 100644 --- a/lib/services/local/bot_instance_test.go +++ b/lib/services/local/bot_instance_test.go @@ -159,7 +159,7 @@ func createInstances(t *testing.T, ctx context.Context, service *BotInstanceServ } // listInstances fetches all instances from the BotInstanceService matching the botName filter -func listInstances(t *testing.T, ctx context.Context, service *BotInstanceService, botName string, searchTerm string) []*machineidv1.BotInstance { +func listInstances(t *testing.T, ctx context.Context, service *BotInstanceService, botName string, searchTerm string, sort *types.SortBy) []*machineidv1.BotInstance { t.Helper() var resources []*machineidv1.BotInstance @@ -168,7 +168,7 @@ func listInstances(t *testing.T, ctx context.Context, service *BotInstanceServic var err error for { - bis, nextKey, err = service.ListBotInstances(ctx, botName, 0, nextKey, searchTerm) + bis, nextKey, err = service.ListBotInstances(ctx, botName, 0, nextKey, searchTerm, sort) require.NoError(t, err) resources = append(resources, bis...) @@ -307,7 +307,7 @@ func TestBotInstanceCRUD(t *testing.T) { require.EqualExportedValues(t, patched, bi2) require.Equal(t, bi.Metadata.Name, bi2.Metadata.Name) - resources := listInstances(t, ctx, service, "example", "") + resources := listInstances(t, ctx, service, "example", "", nil) require.Len(t, resources, 1, "must list only 1 bot instance") require.EqualExportedValues(t, patched, resources[0]) @@ -354,14 +354,14 @@ func TestBotInstanceList(t *testing.T) { bIds := createInstances(t, ctx, service, "b", 4) // listing "a" should only return known "a" instances - aInstances := listInstances(t, ctx, service, "a", "") + aInstances := listInstances(t, ctx, service, "a", "", nil) require.Len(t, aInstances, 3) for _, ins := range aInstances { require.Contains(t, aIds, ins.Spec.InstanceId) } // listing "b" should only return known "b" instances - bInstances := listInstances(t, ctx, service, "b", "") + bInstances := listInstances(t, ctx, service, "b", "", nil) require.Len(t, bInstances, 4) for _, ins := range bInstances { require.Contains(t, bIds, ins.Spec.InstanceId) @@ -376,7 +376,7 @@ func TestBotInstanceList(t *testing.T) { } // Listing an empty bot name ("") should return all instances. - allInstances := listInstances(t, ctx, service, "", "") + allInstances := listInstances(t, ctx, service, "", "", nil) require.Len(t, allInstances, 7) for _, ins := range allInstances { require.Contains(t, allIds, ins.Spec.InstanceId) @@ -445,10 +445,38 @@ func TestBotInstanceListWithSearchFilter(t *testing.T) { _, err = service.CreateBotInstance(ctx, newBotInstance("bot-not-matched")) require.NoError(t, err) - instances := listInstances(t, ctx, service, "", tc.searchTerm) + instances := listInstances(t, ctx, service, "", tc.searchTerm, nil) require.Len(t, instances, 1) require.Equal(t, tc.instance.Spec.InstanceId, instances[0].Spec.InstanceId) }) } } + +// TestBotInstanceListWithSort verifies sorting returns a not-implemented error. +func TestBotInstanceListWithSort(t *testing.T) { + t.Parallel() + + clock := clockwork.NewFakeClock() + + ctx := context.Background() + + mem, err := memory.New(memory.Config{ + Context: ctx, + Clock: clock, + }) + require.NoError(t, err) + + service, err := NewBotInstanceService(backend.NewSanitizer(mem), clock) + require.NoError(t, err) + + _, _, err = service.ListBotInstances(ctx, "", 0, "", "", &types.SortBy{ + Field: "test_field", + IsDesc: true, + }) + require.Error(t, err) + require.Equal(t, "unsupported sort, only bot_name:asc is supported, but got \"test_field\" (desc = true)", err.Error()) + + _, _, err = service.ListBotInstances(ctx, "", 0, "", "", nil) + require.NoError(t, err) +} diff --git a/lib/web/machineid.go b/lib/web/machineid.go index a5bddfdc7d304..d014f21f1eb4c 100644 --- a/lib/web/machineid.go +++ b/lib/web/machineid.go @@ -317,11 +317,19 @@ func (h *Handler) listBotInstances(_ http.ResponseWriter, r *http.Request, _ htt } } + var sort *types.SortBy + if r.URL.Query().Has("sort") { + sortString := r.URL.Query().Get("sort") + s := types.GetSortByFromString(sortString) + sort = &s + } + instances, err := clt.BotInstanceServiceClient().ListBotInstances(r.Context(), &machineidv1.ListBotInstancesRequest{ FilterBotName: r.URL.Query().Get("bot_name"), PageSize: int32(pageSize), PageToken: r.URL.Query().Get("page_token"), FilterSearchTerm: r.URL.Query().Get("search"), + Sort: sort, }) if err != nil { return nil, trace.Wrap(err) diff --git a/web/packages/design/src/DataTable/sort.test.ts b/web/packages/design/src/DataTable/sort.test.ts new file mode 100644 index 0000000000000..41783d0dc3bb2 --- /dev/null +++ b/web/packages/design/src/DataTable/sort.test.ts @@ -0,0 +1,50 @@ +/** + * Teleport + * Copyright (C) 2025 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { formatSortType, parseSortType } from './sort'; + +describe('parseSortType', () => { + it.each` + input | expected + ${'name:asc'} | ${{ fieldName: 'name', dir: 'ASC' }} + ${'name:desc'} | ${{ fieldName: 'name', dir: 'DESC' }} + ${'NAME:asc'} | ${{ fieldName: 'NAME', dir: 'ASC' }} + ${'NAME:ASC'} | ${{ fieldName: 'NAME', dir: 'ASC' }} + ${''} | ${null} + ${'name'} | ${{ fieldName: 'name', dir: 'ASC' }} + ${':asc'} | ${null} + ${':desc'} | ${null} + ${'name:asc:blah'} | ${{ fieldName: 'name', dir: 'ASC' }} + ${'name:desc:blah'} | ${{ fieldName: 'name', dir: 'DESC' }} + ${'name:blah'} | ${{ fieldName: 'name', dir: 'ASC' }} + `('should parse "$input"', ({ input, expected }) => { + expect(parseSortType(input)).toEqual(expected); + }); +}); + +describe('formatSortType', () => { + it.each` + input | expected + ${{ fieldName: 'name', dir: 'ASC' }} | ${'name:asc'} + ${{ fieldName: 'name', dir: 'DESC' }} | ${'name:desc'} + ${{ fieldName: '', dir: 'ASC' }} | ${':asc'} + ${{ fieldName: 'UPPER', dir: 'ASC' }} | ${'UPPER:asc'} + `('should format $input', ({ input, expected }) => { + expect(formatSortType(input)).toEqual(expected); + }); +}); diff --git a/web/packages/design/src/DataTable/sort.ts b/web/packages/design/src/DataTable/sort.ts new file mode 100644 index 0000000000000..e106037b79b60 --- /dev/null +++ b/web/packages/design/src/DataTable/sort.ts @@ -0,0 +1,43 @@ +/** + * Teleport + * Copyright (C) 2025 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { SortType } from './types'; + +/** + * parseSortType converts the "fieldname:dir" format into {fieldName: "", dir: ""} + * @param sort the sort string to parse + * @returns the parsed sort type or null if the sort string is invalid + */ +export function parseSortType(sort: string): SortType | null { + if (!sort) return null; + const [fieldName, dir] = sort.split(':'); + if (!fieldName) return null; + return { + fieldName, + dir: dir?.toLowerCase() === 'desc' ? 'DESC' : 'ASC', + }; +} + +/** + * formatSortType converts a SortType ({fieldName: "", dir: ""}) to the "fieldname:dir" format + * @param sortType the sort to format + * @returns the formatted sort string + */ +export function formatSortType(sortType: SortType): string { + return `${sortType.fieldName}:${sortType.dir.toLowerCase()}`; +} diff --git a/web/packages/teleport/src/BotInstances/BotInstances.test.tsx b/web/packages/teleport/src/BotInstances/BotInstances.test.tsx index c1aa62ed625c6..ab8cad2387475 100644 --- a/web/packages/teleport/src/BotInstances/BotInstances.test.tsx +++ b/web/packages/teleport/src/BotInstances/BotInstances.test.tsx @@ -34,12 +34,15 @@ import { } from 'design/utils/testing'; import { InfoGuidePanelProvider } from 'shared/components/SlidingSidePanel/InfoGuide'; +import { createTeleportContext } from 'teleport/mocks/contexts'; import { listBotInstances } from 'teleport/services/bot/bot'; +import { makeAcl } from 'teleport/services/user/makeAcl'; import { listBotInstancesError, listBotInstancesSuccess, } from 'teleport/test/helpers/botInstances'; +import { ContextProvider } from '..'; import { BotInstances } from './BotInstances'; jest.mock('teleport/services/bot/bot', () => { @@ -78,7 +81,7 @@ describe('BotInstances', () => { }) ); - render(, { wrapper: Wrapper }); + render(, { wrapper: makeWrapper() }); await waitForElementToBeRemoved(() => screen.queryByTestId('loading')); @@ -91,15 +94,67 @@ describe('BotInstances', () => { }); it('Shows an error state', async () => { - server.use(listBotInstancesError(500)); + server.use(listBotInstancesError(500, 'server error')); - render(, { wrapper: Wrapper }); + render(, { wrapper: makeWrapper() }); await waitForElementToBeRemoved(() => screen.queryByTestId('loading')); + expect(screen.getByText('Error: server error')).toBeInTheDocument(); + }); + + it('Shows an unsupported sort error state', async () => { + const testErrorMessage = + 'unsupported sort, only bot_name:asc is supported, but got "blah" (desc = true)'; + server.use(listBotInstancesError(400, testErrorMessage)); + + render(, { wrapper: makeWrapper() }); + + await waitForElementToBeRemoved(() => screen.queryByTestId('loading')); + + expect(screen.getByText(`Error: ${testErrorMessage}`)).toBeInTheDocument(); + + server.use( + listBotInstancesSuccess({ + bot_instances: [], + next_page_token: '', + }) + ); + + const resetButton = screen.getByText('Reset sort'); + expect(resetButton).toBeInTheDocument(); + fireEvent.click(resetButton); + + await waitForElementToBeRemoved(() => screen.queryByTestId('loading')); + + expect( + screen.queryByText(`Error: ${testErrorMessage}`) + ).not.toBeInTheDocument(); + }); + + it('Shows an unauthorised error state', async () => { + render(, { + wrapper: makeWrapper( + makeAcl({ + botInstances: { + list: false, + create: true, + edit: true, + remove: true, + read: true, + }, + }) + ), + }); + expect( - screen.getByText('Error: 500', { exact: false }) + screen.getByText( + 'You do not have permission to access Bot instances. Missing role permissions:', + { exact: false } + ) ).toBeInTheDocument(); + + expect(screen.getByText('bot_instance.list')).toBeInTheDocument(); }); it('Shows a list', async () => { @@ -123,7 +178,7 @@ describe('BotInstances', () => { }) ); - render(, { wrapper: Wrapper }); + render(, { wrapper: makeWrapper() }); await waitForElementToBeRemoved(() => screen.queryByTestId('loading')); @@ -157,7 +212,7 @@ describe('BotInstances', () => { expect(listBotInstances).toHaveBeenCalledTimes(0); - render(, { wrapper: Wrapper }); + render(, { wrapper: makeWrapper() }); await waitForElementToBeRemoved(() => screen.queryByTestId('loading')); @@ -168,6 +223,7 @@ describe('BotInstances', () => { pageSize: 20, pageToken: '', searchTerm: '', + sort: 'active_at_latest:desc', }); await waitFor(() => expect(nextButton).toBeEnabled()); @@ -178,6 +234,7 @@ describe('BotInstances', () => { pageSize: 20, pageToken: '.next', searchTerm: '', + sort: 'active_at_latest:desc', }); await waitFor(() => expect(nextButton).toBeEnabled()); @@ -188,6 +245,7 @@ describe('BotInstances', () => { pageSize: 20, pageToken: '.next.next', searchTerm: '', + sort: 'active_at_latest:desc', }); const [prevButton] = screen.getAllByTitle('Previous page'); @@ -227,7 +285,7 @@ describe('BotInstances', () => { expect(listBotInstances).toHaveBeenCalledTimes(0); - render(, { wrapper: Wrapper }); + render(, { wrapper: makeWrapper() }); await waitForElementToBeRemoved(() => screen.queryByTestId('loading')); @@ -236,6 +294,7 @@ describe('BotInstances', () => { pageSize: 20, pageToken: '', searchTerm: '', + sort: 'active_at_latest:desc', }); const [nextButton] = screen.getAllByTitle('Next page'); @@ -247,6 +306,7 @@ describe('BotInstances', () => { pageSize: 20, pageToken: '.next', searchTerm: '', + sort: 'active_at_latest:desc', }); jest.useRealTimers(); // Required as userEvent.type() uses setTimeout internally @@ -261,20 +321,94 @@ describe('BotInstances', () => { pageSize: 20, pageToken: '', // Search should reset to the first page searchTerm: 'test-search-term', + sort: 'active_at_latest:desc', + }); + }); + + it('Allows sorting', async () => { + jest.mocked(listBotInstances).mockImplementation( + ({ pageToken }) => + new Promise(resolve => { + resolve({ + bot_instances: [ + { + bot_name: `test-bot`, + instance_id: `00000000-0000-4000-0000-000000000000`, + active_at_latest: `2025-05-19T07:32:00Z`, + host_name_latest: 'test-hostname', + join_method_latest: 'test-join-method', + version_latest: `1.0.0-dev-a12b3c`, + }, + ], + next_page_token: pageToken + '.next', + }); + }) + ); + + expect(listBotInstances).toHaveBeenCalledTimes(0); + + render(, { wrapper: makeWrapper() }); + + await waitForElementToBeRemoved(() => screen.queryByTestId('loading')); + + const lastHeartbeatHeader = screen.getByText('Last heartbeat'); + + expect(listBotInstances).toHaveBeenCalledTimes(1); + expect(listBotInstances).toHaveBeenLastCalledWith({ + pageSize: 20, + pageToken: '', + searchTerm: '', + sort: 'active_at_latest:desc', + }); + + fireEvent.click(lastHeartbeatHeader); + + expect(listBotInstances).toHaveBeenCalledTimes(2); + expect(listBotInstances).toHaveBeenLastCalledWith({ + pageSize: 20, + pageToken: '', + searchTerm: '', + sort: 'active_at_latest:asc', + }); + + const botHeader = screen.getByText('Bot'); + fireEvent.click(botHeader); + + expect(listBotInstances).toHaveBeenCalledTimes(3); + expect(listBotInstances).toHaveBeenLastCalledWith({ + pageSize: 20, + pageToken: '', + searchTerm: '', + sort: 'bot_name:desc', }); }); }); -function Wrapper({ children }: PropsWithChildren) { - return ( - - - - - {children} - - - - - ); +function makeWrapper( + customAcl: ReturnType = makeAcl({ + botInstances: { + list: true, + create: true, + edit: true, + remove: true, + read: true, + }, + }) +) { + return ({ children }: PropsWithChildren) => { + const ctx = createTeleportContext({ + customAcl, + }); + return ( + + + + + {children} + + + + + ); + }; } diff --git a/web/packages/teleport/src/BotInstances/BotInstances.tsx b/web/packages/teleport/src/BotInstances/BotInstances.tsx index 1d5b25a6df94e..f8b1e64dcf293 100644 --- a/web/packages/teleport/src/BotInstances/BotInstances.tsx +++ b/web/packages/teleport/src/BotInstances/BotInstances.tsx @@ -17,11 +17,13 @@ */ import { keepPreviousData, useQuery } from '@tanstack/react-query'; -import { useCallback } from 'react'; +import { useCallback, useMemo } from 'react'; import { useHistory, useLocation } from 'react-router'; import { Alert } from 'design/Alert/Alert'; import Box from 'design/Box/Box'; +import { formatSortType, parseSortType } from 'design/DataTable/sort'; +import { SortType } from 'design/DataTable/types'; import { Indicator } from 'design/Indicator/Indicator'; import { Mark } from 'design/Mark/Mark'; import { @@ -31,6 +33,7 @@ import { ReferenceLinks, } from 'shared/components/SlidingSidePanel/InfoGuide/InfoGuide'; +import { EmptyState } from 'teleport/Bots/List/EmptyState/EmptyState'; import { FeatureBox, FeatureHeader, @@ -39,28 +42,37 @@ import { import cfg from 'teleport/config'; import { listBotInstances } from 'teleport/services/bot/bot'; import { BotInstanceSummary } from 'teleport/services/bot/types'; +import useTeleport from 'teleport/useTeleport'; import { BotInstancesList } from './List/BotInstancesList'; export function BotInstances() { const history = useHistory(); - const location = useLocation(); + const location = useLocation<{ prevPageTokens?: readonly string[] }>(); const queryParams = new URLSearchParams(location.search); const pageToken = queryParams.get('page') ?? ''; const searchTerm = queryParams.get('search') ?? ''; + const sort = queryParams.get('sort') || 'active_at_latest:desc'; + + const ctx = useTeleport(); + const flags = ctx.getFeatureFlags(); + const canListInstances = flags.listBotInstances; const { isPending, isFetching, isSuccess, isError, error, data } = useQuery({ - queryKey: ['bot_instances', 'list', searchTerm, pageToken], + enabled: canListInstances, + queryKey: ['bot_instances', 'list', searchTerm, pageToken, sort], queryFn: () => listBotInstances({ pageSize: 20, pageToken, searchTerm, + sort, }), placeholderData: keepPreviousData, staleTime: 30_000, // Cached pages are valid for 30 seconds }); + const { prevPageTokens = [] } = location.state ?? {}; const hasNextPage = !!data?.next_page_token; const hasPrevPage = !!pageToken; @@ -68,26 +80,55 @@ export function BotInstances() { const search = new URLSearchParams(location.search); search.set('page', data?.next_page_token ?? ''); - history.push({ - pathname: `${location.pathname}`, - search: search.toString(), - }); - }, [data?.next_page_token, history, location.pathname, location.search]); + history.replace( + { + pathname: location.pathname, + search: search.toString(), + }, + { + prevPageTokens: [...prevPageTokens, pageToken], + } + ); + }, [ + data?.next_page_token, + history, + location.pathname, + location.search, + pageToken, + prevPageTokens, + ]); const handleFetchPrev = useCallback(() => { - history.goBack(); - }, [history]); + const prevTokens = [...prevPageTokens]; + const nextToken = prevTokens.pop(); - const handleSearchChange = useCallback((term: string) => { const search = new URLSearchParams(location.search); - search.set('search', term); - search.set('page', ''); + search.set('page', nextToken ?? ''); + + history.replace( + { + pathname: location.pathname, + search: search.toString(), + }, + { + prevPageTokens: prevTokens, + } + ); + }, [history, location.pathname, location.search, prevPageTokens]); + + const handleSearchChange = useCallback( + (term: string) => { + const search = new URLSearchParams(location.search); + search.set('search', term); + search.set('page', ''); - history.push({ - pathname: `${location.pathname}`, - search: search.toString(), - }); - }, []); + history.replace({ + pathname: `${location.pathname}`, + search: search.toString(), + }); + }, + [history, location.pathname, location.search] + ); const onItemSelected = useCallback( (item: BotInstanceSummary) => { @@ -101,6 +142,38 @@ export function BotInstances() { [history] ); + const sortType = useMemo(() => parseSortType(sort), [sort]); + + const handleSortChanged = useCallback( + (sortType: SortType) => { + const formattedSortType = formatSortType(sortType); + + const search = new URLSearchParams(location.search); + search.set('sort', formattedSortType); + search.set('page', ''); + + history.replace({ + pathname: location.pathname, + search: search.toString(), + }); + }, + [history, location.pathname, location.search] + ); + + const hasUnsupportedSortError = isUnsupportedSortError(error); + + if (!canListInstances) { + return ( + + + You do not have permission to access Bot instances. Missing role + permissions: bot_instance.list + + + + ); + } + return ( @@ -114,7 +187,21 @@ export function BotInstances() { ) : undefined} - {isError ? ( + {isError && hasUnsupportedSortError ? ( + { + handleSortChanged({ fieldName: 'bot_name', dir: 'ASC' }); + }, + }} + > + {`Error: ${error.message}`} + + ) : undefined} + + {isError && !hasUnsupportedSortError ? ( {`Error: ${error.message}`} ) : undefined} @@ -127,6 +214,8 @@ export function BotInstances() { onSearchChange={handleSearchChange} searchTerm={searchTerm} onItemSelected={onItemSelected} + sortType={sortType} + onSortChanged={handleSortChanged} /> ) : undefined} @@ -186,3 +275,7 @@ const InfoGuideReferenceLinks = { href: 'https://goteleport.com/docs/reference/cli/tctl/#tctl-bots-instances-add', }, }; + +const isUnsupportedSortError = (error: Error) => { + return error?.message && error.message.includes('unsupported sort'); +}; diff --git a/web/packages/teleport/src/BotInstances/List/BotInstancesList.tsx b/web/packages/teleport/src/BotInstances/List/BotInstancesList.tsx index 75cb02be09ef6..cc192727dc29b 100644 --- a/web/packages/teleport/src/BotInstances/List/BotInstancesList.tsx +++ b/web/packages/teleport/src/BotInstances/List/BotInstancesList.tsx @@ -25,7 +25,7 @@ import styled from 'styled-components'; import { Info } from 'design/Alert/Alert'; import { Cell, LabelCell } from 'design/DataTable/Cells'; import Table from 'design/DataTable/Table'; -import { FetchingConfig } from 'design/DataTable/types'; +import { FetchingConfig, SortType } from 'design/DataTable/types'; import Flex from 'design/Flex'; import Text from 'design/Text'; import { HoverTooltip } from 'design/Tooltip/HoverTooltip'; @@ -46,18 +46,22 @@ export function BotInstancesList({ searchTerm, onSearchChange, onItemSelected, + sortType, + onSortChanged, }: { data: BotInstanceSummary[]; searchTerm: string; onSearchChange: (term: string) => void; onItemSelected: (item: BotInstanceSummary) => void; + sortType: SortType; + onSortChanged: (sortType: SortType) => void; } & Omit) { const tableData = data.map(x => ({ ...x, hostnameDisplay: x.host_name_latest ?? '-', instanceIdDisplay: x.instance_id.substring(0, 7), versionDisplay: x.version_latest ? `v${x.version_latest}` : '-', - activeAtDisplay: x.active_at_latest + active_at_latest: x.active_at_latest ? `${formatDistanceToNowStrict(parseISO(x.active_at_latest))} ago` : '-', activeAtLocal: x.active_at_latest @@ -83,8 +87,8 @@ export function BotInstancesList({ disableLoadingIndicator: true, }} serversideProps={{ - sort: undefined, - setSort: () => undefined, + sort: sortType, + setSort: onSortChanged, serversideSearchPanel: ( ( + isSortable: true, + render: ({ active_at_latest, activeAtLocal }) => ( - - - {activeAtDisplay} - - + + {active_at_latest} + ), }, diff --git a/web/packages/teleport/src/components/hooks/useUrlFiltering/encodeUrlQueryParams.ts b/web/packages/teleport/src/components/hooks/useUrlFiltering/encodeUrlQueryParams.ts index fbcbf0217f917..d755598bcfa81 100644 --- a/web/packages/teleport/src/components/hooks/useUrlFiltering/encodeUrlQueryParams.ts +++ b/web/packages/teleport/src/components/hooks/useUrlFiltering/encodeUrlQueryParams.ts @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +import { formatSortType } from 'design/DataTable/sort'; import { SortType } from 'design/DataTable/types'; export type EncodeUrlQueryParamsProps = { @@ -42,7 +43,7 @@ export function encodeUrlQueryParams({ } if (sort) { - urlParams.append('sort', `${sort.fieldName}:${sort.dir.toLowerCase()}`); + urlParams.append('sort', formatSortType(sort)); } if (pinnedOnly !== undefined) { diff --git a/web/packages/teleport/src/components/hooks/useUrlFiltering/useUrlFiltering.ts b/web/packages/teleport/src/components/hooks/useUrlFiltering/useUrlFiltering.ts index d305cbd7d54fb..661bb3008a2bd 100644 --- a/web/packages/teleport/src/components/hooks/useUrlFiltering/useUrlFiltering.ts +++ b/web/packages/teleport/src/components/hooks/useUrlFiltering/useUrlFiltering.ts @@ -19,6 +19,7 @@ import { useMemo, useState } from 'react'; import { useLocation } from 'react-router'; +import { parseSortType } from 'design/DataTable/sort'; import { SortType } from 'design/DataTable/types'; import { IncludedResourceMode } from 'shared/components/UnifiedResources'; import { makeAdvancedSearchQueryForLabel } from 'shared/utils/advancedSearchLabelQuery'; @@ -130,15 +131,7 @@ export default function getResourceUrlQueryParams( const sort = searchParams.get('sort'); const kinds = searchParams.has('kinds') ? searchParams.getAll('kinds') : null; - const sortParam = sort ? sort.split(':') : null; - - // Converts the "fieldname:dir" format into {fieldName: "", dir: ""} - const processedSortParam = sortParam - ? ({ - fieldName: sortParam[0], - dir: sortParam[1]?.toUpperCase() || 'ASC', - } as SortType) - : null; + const processedSortParam = parseSortType(sort); return { query, diff --git a/web/packages/teleport/src/services/bot/bot.ts b/web/packages/teleport/src/services/bot/bot.ts index 8a4b0e9154e59..a9631183ce03a 100644 --- a/web/packages/teleport/src/services/bot/bot.ts +++ b/web/packages/teleport/src/services/bot/bot.ts @@ -138,11 +138,12 @@ export async function listBotInstances( pageToken: string; pageSize: number; searchTerm?: string; + sort?: string; botName?: string; }, signal?: AbortSignal ) { - const { pageToken, pageSize, searchTerm, botName } = variables; + const { pageToken, pageSize, searchTerm, sort, botName } = variables; const path = cfg.listBotInstancesUrl(); const qs = new URLSearchParams(); @@ -152,6 +153,9 @@ export async function listBotInstances( if (searchTerm) { qs.set('search', searchTerm); } + if (sort) { + qs.set('sort', sort); + } if (botName) { qs.set('bot_name', botName); } diff --git a/web/packages/teleport/src/test/helpers/botInstances.ts b/web/packages/teleport/src/test/helpers/botInstances.ts index 13034b2a8936d..fb7d221f447aa 100644 --- a/web/packages/teleport/src/test/helpers/botInstances.ts +++ b/web/packages/teleport/src/test/helpers/botInstances.ts @@ -34,9 +34,9 @@ export const listBotInstancesSuccess = (mock: ListBotInstancesResponse) => return HttpResponse.json(mock); }); -export const listBotInstancesError = (status: number) => +export const listBotInstancesError = (status: number, error: string = null) => http.get(listBotInstancesPath, () => { - return new HttpResponse(null, { status }); + return HttpResponse.json({ error: { message: error } }, { status }); }); export const getBotInstanceSuccess = (mock: GetBotInstanceResponse) =>